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 SQL Server + triggers breaking change #3887

Merged
merged 1 commit into from
Jun 13, 2022

Conversation

roji
Copy link
Member

@roji roji commented Jun 9, 2022

Closes #3883

@roji roji marked this pull request as ready for review June 9, 2022 09:14
@roji roji requested a review from a team June 9, 2022 09:14
@roji
Copy link
Member Author

roji commented Jun 9, 2022

@ErikEJ
Copy link
Contributor

ErikEJ commented Jun 9, 2022

Could it be just HasTriggers() / HasTrigger() (inSQL Server there can be multiple)

@roji
Copy link
Member Author

roji commented Jun 9, 2022

The idea was for this to be the extension point for a future actual trigger API, which is why we didn't want an API that was only usable to declare that some trigger(s) are present. But FWIW I agree it's a bit odd.

@ajcvickers
Copy link
Member

ajcvickers commented Jun 13, 2022

@roji @ErikEJ I think we should re-discuss this. It's weird having to specify a meaningless name, and I don't really see the issue with "an API that was only usable to declare that some trigger(s) are present."

Well, I guess it should not "only be usable for that", but being "usable for that" seems fine.

@roji
Copy link
Member Author

roji commented Jun 13, 2022

@ajcvickers note that there are some other (relatively exotic) edge cases where OUTPUT (without INTO) isn't allowed, which have nothing to do with triggers. At some point I proposed another SQL Server-specific metadata API which would directly tell EF to avoid OUTPUT without INTO (UseUniversalUpdateTechnique?), and which a future HasTrigger API would turn on by convention; but we decided against this in design. We can revisit.

@roji roji merged commit af1a4ee into dotnet:main Jun 13, 2022
@roji roji deleted the SqlServerTriggers branch June 13, 2022 11:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Document configuration of triggers for SaveChanges
3 participants