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

SqlServerValueGenerationStrategy which conflicts now causes warning #19887

Merged
merged 3 commits into from
Feb 13, 2020

Conversation

lajones
Copy link
Contributor

@lajones lajones commented Feb 12, 2020

Instead of throwing as it used to do. This fixes #16814.

By default the warning is still set up to throw, but now you could turn that off if you're sure you know what you're doing. Tests for that latter part will be forthcoming, but I wanted to get feedback that I'm going the right direction, plus agree on the message etc.

@lajones lajones self-assigned this Feb 12, 2020
@@ -241,4 +241,8 @@
<data name="FunctionOnClient" xml:space="preserve">
<value>The '{methodName}' method is not supported because the query has switched to client-evaluation. Inspect the log to determine which query expressions are triggering client-evaluation.</value>
</data>
<data name="LogConflictingValueGenerationStrategies" xml:space="preserve">
<value>Both the SqlServerValueGenerationStrategy {generationStrategy} and {otherGenerationStrategy} have been set on property '{propertyName}' on entity '{entityName}'. Usually this is a mistake. Only use these at the same time if you are sure you understand the consequences.</value>
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
<value>Both the SqlServerValueGenerationStrategy {generationStrategy} and {otherGenerationStrategy} have been set on property '{propertyName}' on entity '{entityName}'. Usually this is a mistake. Only use these at the same time if you are sure you understand the consequences.</value>
<value>Both the SqlServerValueGenerationStrategy {generationStrategy} and {otherGenerationStrategy} have been set on property '{propertyName}' on entity type '{entityName}'. Usually this is a mistake. Only use these at the same time if you are sure you understand the consequences.</value>

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Looks good. Will do.

@lajones
Copy link
Contributor Author

lajones commented Feb 13, 2020

@AndriySvyryd Added 2 functional tests that it a) throws by default, b) does not throw but just logs the warning if you override the default.

@lajones lajones merged commit e3f6eee into master Feb 13, 2020
@lajones lajones deleted the 20200210_Issue16814_01 branch February 13, 2020 20:51
@lajones lajones added the closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. label Feb 13, 2020
@lajones lajones added this to the 5.0.0 milestone Feb 13, 2020
@lajones lajones removed the closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. label Feb 13, 2020
@lajones lajones removed this from the 5.0.0 milestone Feb 13, 2020
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.

Allow DefaultValueSql when using ForSqlServerUseSequenceHiLo
2 participants