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

Some fixes to SqlQueryTestBase #30384

Merged
merged 1 commit into from
Mar 2, 2023
Merged

Some fixes to SqlQueryTestBase #30384

merged 1 commit into from
Mar 2, 2023

Conversation

roji
Copy link
Member

@roji roji commented Mar 2, 2023

@roji roji requested a review from ajcvickers March 2, 2023 11:04
@roji roji changed the title Normalize delimiters in raw string Normalize delimiters in tests Mar 2, 2023
@roji roji marked this pull request as ready for review March 2, 2023 11:39
@roji
Copy link
Member Author

roji commented Mar 2, 2023

@ajcvickers made a couple more changes there, it's a bit odd we don't actually override NormalizeDelimiters* for SQL Server to not change the brackets to double-quotes (but SQL Server also accepts double-quotes so it works)

@ajcvickers
Copy link
Member

Why is that odd?

@roji
Copy link
Member Author

roji commented Mar 2, 2023

Well we generally use brackets everywhere for SQL Server, but not here. That means we get mixed queries out, with SQL fragments using double-quotes and EF-generated SQL using brackets. Also it begs the question of why the SQL in the tests uses brackets in the first place, when no provider actually preserves them.

@ajcvickers
Copy link
Member

@roji I misread your first comment. Yeah, agree that is odd. I would expect us to always use brackets on SQL Server.

@roji roji mentioned this pull request Mar 2, 2023
4 tasks
@roji roji changed the title Normalize delimiters in tests Some fixes to SqlQueryTestBase Mar 2, 2023
@roji roji merged commit 27dd0e7 into dotnet:main Mar 2, 2023
@roji roji deleted the TestStuff branch March 2, 2023 12:43
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.

None yet

2 participants