TrimStart and TrimEnd with optional char array implementation for SQL Server#33715
TrimStart and TrimEnd with optional char array implementation for SQL Server#33715maumar merged 14 commits intodotnet:mainfrom
Conversation
|
@dotnet-policy-service agree |
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
# Conflicts: # src/EFCore.SqlServer/Query/Internal/SqlServerMethodCallTranslatorProvider.cs
…RTRIM. Handling compatibility level to low." This reverts commit e1a73e7.
…d end in a consistent way.
# Conflicts: # src/EFCore.SqlServer/Query/Internal/SqlServerMethodCallTranslatorProvider.cs
Yes, I saw the Trim, but since it has a different syntax in the new version, it is unlikely that it can be done in the same way. So, @roji, if you agree, I would prefer to address it after finishing these. For some reason, my SQLite tests are failing again. Maybe I'm doing something wrong in the merge or something? |
|
Commenter does not have sufficient privileges for PR 33715 in repo dotnet/efcore |
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
…lidated by compatibility level.
Head branch was pushed to by a user without write access
roji
left a comment
There was a problem hiding this comment.
Thanks, this is very close - see note below. I'll just go ahead and fix the remaining issue plus some cleanup.
Thanks for your contribution!
| || (TrimStartMethodInfoWithCharArrayArg.Equals(method) | ||
| // SqlServer LTRIM does not take arguments | ||
| && ((arguments[0] as SqlConstantExpression)?.Value as Array)?.Length == 0)) | ||
| || (_sqlServerSingletonOptions.CompatibilityLevel >= 160 |
There was a problem hiding this comment.
It's a bit of an annoying little detail, but the previous code did translate TrimStartMethodInfoWithCharArrayArg as long as the first argument is an empty array constant. I don't think it was an important thing to do, but once it's there I don't think we should be removing it (it's a very minor but also needless breaking change). So can we please preserve the previous behavior, and only put the new translation inside the CompatibilityLevel condition?
dotnet#33715 and dotnet#34002 were developed concurrently. Their merge does not build because of some changes in the types returned by `ISqlExpressionFactory`.
Fixes #22924