Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

Added AppendStoredProcedureCall#1185

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Open
Camel-RD wants to merge6 commits intoFirebirdSQL:master
base:master
Choose a base branch
Loading
fromCamel-RD:NETProvider-b1

Conversation

@Camel-RD
Copy link

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

@cincuranet
Copy link
Member

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 inE2E tests.

@Camel-RD
Copy link
Author

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
Copy link
Author

AppendStoredProcedureCall fixed and tests added

Copy link
Member

@cincuranetcincuranet left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

Some minor pieces, otherwise LGTM in general.

}
}

commandStringBuilder.Append("SELECT * FROM ");
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

I would prefer here to be explicit about column names.

@Camel-RD
Copy link
Author

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
Copy link
Member

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

@cincuranet
Copy link
Member

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
Copy link
Author

Camel-RD commentedSep 25, 2024
edited
Loading

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
Copy link
Member

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

SQL Server implementation usesEXEC 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
Copy link
Author

Camel-RD commentedSep 25, 2024
edited
Loading

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 usesEXEC 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.

Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment

Reviewers

@cincuranetcincuranetcincuranet requested changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Is InsertUsingStoredProcedure, UpdateUsingStoredProcedure, DeleteUsingStoredProcedure in model builder supported?

2 participants

@Camel-RD@cincuranet

[8]ページ先頭

©2009-2025 Movatter.jp