Added AppendStoredProcedureCall

[Camel-RD]

Added AppendStoredProcedureCall override to support InsertUsingStoredProcedure, UpdateUsingStoredProcedure, DeleteUsingStoredProcedure in model builder.

[cincuranet]

Sorry for late reply, I was busy with work. Can you maybe add some simple tests for calling insert/update/delete SPs, just to have basic validation that the SQL is correct. Something similar to what is in E2E tests.

[Camel-RD]

It looks like EXECUTE PROCEDURE SP_ABC p1, p2 will not work properly with EF concurrency checks. Will have to change it to SELECT * FROM SP_ABC(p1, p2, p3).

[Camel-RD]

AppendStoredProcedureCall fixed and tests added

[cincuranet]

Some minor pieces, otherwise LGTM in general.

[cincuranet]

I would prefer here to be explicit about column names.

[Camel-RD]

Explicit column names means that we cannot freely choose the results column names in SPs, but we can pick names for input parameters. Also, EFC uses result values by their positions and ignores any given result column names.

[cincuranet]

Not sure why you added commit 53fccb6 , but that looks unrelated and needs to be removed.

[cincuranet]

Explicit column names means that we cannot freely choose the results column names in SPs, but we can pick names for input parameters. Also, EFC uses result values by their positions and ignores any given result column names.

I don't follow. Can you elaborate?

[Camel-RD]

Explicit column names means that we cannot freely choose the results column names in SPs, but we can pick names for input parameters. Also, EFC uses result values by their positions and ignores any given result column names.

I don't follow. Can you elaborate?

Not to use explicit names is how it's done in SQL Server implementation.

If explicit column names are used, there would also be an issue with naming the column for the rows affected value. The name could be ROWSCOUNT, but it should be properly documented so developers writing stored procedures can easily find this information.

[cincuranet]

Not to use explicit names is how it's done in SQL Server implementation.

SQL Server implementation uses EXEC and SQL Server does not have selectable stored procedures.

If explicit column names are used, there would also be an issue with naming the column for the rows affected value. The name could be ROWSCOUNT, but it should be properly documented so developers writing stored procedures can easily find this information.

Yes. But same can be said about the whole SP structure. It needs to fit into what EF expects.

[Camel-RD]

Updated the code to use explicit column names.

And i saw some additional issues with this AppendStoredProcedureCall code.

When stored procedure doesn't have any return values defined SELECT * FROM SP() will produce an SQL error. In this case EXECUTE PROCEDURE should be used.

Also AppendStoredProcedureCall code should probably throw some exceptions when output parameters or return values are defined.

Made changes to fix these issues and added a test for case when ID isn't auto generetad and insert procedure doesn't return any values.

SQL Server implementation uses EXEC and SQL Server does not have selectable stored procedures.

Hm, SQLServer stored procedures have 3 ways to return values including as selectable procedure.

Yes. But same can be said about the whole SP structure. It needs to fit into what EF expects.

Even if EF Core documentation is lacking, its possible to gues what EF is expecting from exceptions it throws.