Use SQL Server 2025 JSON_VALUE() RETURNING clause #36456
Conversation
roji
force-pushed
the
SqlServerJsonValueReturning
branch
from
4997eeb to
6b62059
Compare
| SqlServerEngineType.SqlServer when SqlServerCompatibilityLevel >= 170 => true, | ||
| SqlServerEngineType.AzureSql when AzureSqlCompatibilityLevel >= 170 => true, | ||
| SqlServerEngineType.AzureSynapse => false, | ||
| _ => false |
There was a problem hiding this comment.
Wouldn't UnreachableException make more sense here?
There was a problem hiding this comment.
The thing is that there's an Unknown value in the SqlServerEngineType enum - any idea in what situation we can get it or what it means?
There was a problem hiding this comment.
The Unknown will never pass through the validation, so it can be ignored here. It's there purely for ConfigureSqlEngine.
There was a problem hiding this comment.
OK, thanks - I'll switch to UnreachableException then.
There was a problem hiding this comment.
Isn't it reachable when the compatibility level is below 170?
There was a problem hiding this comment.
Ah of course :/ Thanks @stevendarby
There was a problem hiding this comment.
Fixed this to throw UnreachableException when an unknown enum label is encountered, but not otherwise.
There was a problem hiding this comment.
@cincuranet the tests failed for me because... in SqlServerSingletonOptions the Engine is Unknown. I'm changing the code to return false for Unknown for now so as to not block on this, but this is something we should fix (opened #36477 to track).
src/EFCore.SqlServer/Query/Internal/SqlServerJsonPostprocessor.cs
Outdated
Show resolved
Hide resolved
roji
force-pushed
the
SqlServerJsonValueReturning
branch
from
6b62059 to
9437369
Compare
|
@cincuranet @stevendarby rebased on latest main and did a couple fixes, should be ready for another review and merge. |
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
3 participants
Based on top of #36442, review last commit only.
Note: disabling this specifically when UseAzureSql() is used, as RETURNING isn't supported there yet (but should be supported very soon).
Closes #35729