Skip to content
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 our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Document the incompatibility of the new 7.0 SaveChanges with certain computed column types #4130

Closed
rkone opened this issue Nov 11, 2022 · 8 comments · Fixed by #4131
Closed

Comments

@rkone
Copy link

rkone commented Nov 11, 2022

After upgrading to EFCore 7.0, I ran into difficulty trying to save an update to my table with a computed column, it's directing me to the following link for further guidance:
https://aka.ms/efcore-docs-sqlserver-save-changes-and-computed-columns
(the link is broken).

This table also has a trigger, giving the table a .HasTrigger("SomeTrigger") configuration has also fixed the computed column error, but I'd like to know when I review / remove the trigger - what should I do about the computed column? I don't see any guidance for it.

@roji
Copy link
Member

roji commented Nov 11, 2022

@rkone EF Core 7.0 uses new, optimized SQL to when inserting new rows; this SQL is incompatible both with triggers and some (rare) types of computed columns. Using HasTrigger() makes EF revert back to the previous SQL, which is compatible everywhere, but significantly slower.

Even if you remove the trigger, you can leave the HasTrigger() as a way to keep to compatible SQL.

@gbiellem
Copy link

Is using HasTrigger() the only way to force the old behavior? I'm uncomfortable coding to a side-effect to workaround this. Also was this change mentioned anywhere in the release notes or is there any doco on it outside of GH issues?

@roji
Copy link
Member

roji commented Nov 14, 2022

Is using HasTrigger() the only way to force the old behavior? I'm uncomfortable coding to a side-effect to workaround this.

Yes, that's currently the only way. This has no other effect other than forcing the old behavior, so it be fine to use it even where a database trigger isn't present.

Also was this change mentioned anywhere in the release notes or is there any doco on it outside of GH issues?

This is fully documented as a breaking change, and also has a dedicated section in the SQL Server provider docs.

@gbiellem
Copy link

@roji Thanks for the info and the prompt reply. Given it's the only way I'll run with HasTrigger().

I respectfully disagree that the impact of the change is fully documented.

I'd already read the links you provided and I just reread them again to be sure I hadn't missed something. Both links discuss triggers; there's nothing there to indicate that this change impacts some types of computed columns. Anybody else who hits this issue is unlikely to solve it based on the current documentation.

@roji
Copy link
Member

roji commented Nov 14, 2022

I'll amend the notes to include the computed column case.

Can you please share your computed column definition, to make sure it aligns with what we know about this limitation? The SQL Server docs specify that the OUTPUT clause cannot be used with:

Subqueries or user-defined functions that perform user or system data access, or are assumed to perform such access. User-defined functions are assumed to perform data access if they are not schema-bound.
A column from a view or inline table-valued function when that column is defined by one of the following methods:

  • A subquery.
  • A user-defined function that performs user or system data access, or is assumed to perform such access.
  • A computed column that contains a user-defined function that performs user or system data access in its definition.

@roji roji transferred this issue from dotnet/efcore Nov 14, 2022
@roji
Copy link
Member

roji commented Nov 14, 2022

Leaving #4129 to track the broken aka.ms link, will use this to track amending the notes.

@roji roji changed the title Save changes and computed columns broken link Document the incompatibility of the new 7.0 SaveChanges with certain computed column types Nov 14, 2022
@roji roji self-assigned this Nov 14, 2022
@roji roji added this to the 7.0.0 milestone Nov 14, 2022
roji added a commit to roji/EntityFramework.Docs that referenced this issue Nov 14, 2022
roji added a commit to roji/EntityFramework.Docs that referenced this issue Nov 14, 2022
@gbiellem
Copy link

gbiellem commented Nov 14, 2022

@roji that aligns with our experience. We've migrated 4 projects from EF6 to EF7. Only the tables that had computed columns calling user defined functions required the HasTrigger() workaround. These UDFs are not schema bound

@rkone
Copy link
Author

rkone commented Nov 14, 2022

We were also in the UDF boat. Thanks for the info and doc updates!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants