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

"Scaffold" triggers for SQL Server #28253

Merged
merged 1 commit into from
Jun 20, 2022
Merged

"Scaffold" triggers for SQL Server #28253

merged 1 commit into from
Jun 20, 2022

Conversation

roji
Copy link
Member

@roji roji commented Jun 16, 2022

HasTrigger with trigger name only, to make the SQL Server SaveChanges work out of the box.

Closes #28185

@roji roji requested a review from a team June 16, 2022 18:55
var table = tables.Single(t => t.Schema == tableSchema && t.Name == tableName);

var triggers = new HashSet<string>();
table[RelationalAnnotationNames.Triggers] = triggers;
Copy link
Member Author

Choose a reason for hiding this comment

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

Note that the annotation value here is HashSet<string>, which is different from what we do via the Fluent API (Dictionary<string, ITrigger>), because ITrigger needs to reference an IMutableEntityType which we don't have here. The translation from this to that happens in RelationalScaffoldingModelFactory.

@roji roji marked this pull request as draft June 16, 2022 21:22
@roji
Copy link
Member Author

roji commented Jun 16, 2022

Scaffolding codegen needs to happen in CSharpDbContextGenerator and not in AnnotationCodeGenerator, since on the model snapshot side special code handles triggers in CSharpSnapshotGenerator. Should have done this after @bricelam's template work :(

@AndriySvyryd
Copy link
Member

:shipit:

@ErikEJ
Copy link
Contributor

ErikEJ commented Jun 17, 2022

LGTM - makes using previews a better experience.

@roji
Copy link
Member Author

roji commented Jun 17, 2022

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@roji roji marked this pull request as ready for review June 17, 2022 07:59
@roji
Copy link
Member Author

roji commented Jun 17, 2022

Ready for review, unrelated Cosmos test failures due to flakiness (:angry:)

@ajcvickers
Copy link
Member

Why are the Cosmos tests running on that build? Did we enable Cosmos there at some point and I forgot about it? @smitpatel?

@ajcvickers
Copy link
Member

Also, it's interesting that it's new tests (Regex_IsMatch_MethodCall...) that are failing. Is there something different about how these tests work than the other tests? I.e. are we sure this is Cosmos flakiness and not something flaky in the new tests?

@roji
Copy link
Member Author

roji commented Jun 17, 2022

Agree that it's strange! Though also not supposed to be related to this PR in any way...

@smitpatel
Copy link
Contributor

Use ConditionalFact

[Fact]
public virtual void Regex_IsMatch_MethodCall_With_Unsupported_Option()
=> Assert.Throws<InvalidOperationException>(() =>
Fixture.CreateContext().Customers.Where(o => Regex.IsMatch(o.CustomerID, "^T", RegexOptions.RightToLeft)).ToList());
[Fact]
public virtual void Regex_IsMatch_MethodCall_With_Any_Unsupported_Option()
=> Assert.Throws<InvalidOperationException>(() =>
Fixture.CreateContext().Customers.Where(o => Regex.IsMatch(o.CustomerID, "^T", RegexOptions.IgnoreCase | RegexOptions.RightToLeft)).ToList());

@roji
Copy link
Member Author

roji commented Jun 17, 2022

@smitpatel did that locally as part of #28139, PR out soon.

HasTrigger with trigger name only, to make the SQL Server SaveChanges work out of the box.

Closes dotnet#28185
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.

SQL Server: Scaffold HasTrigger if the table contains triggers
5 participants