Fix to #30326 - Query: converter from bool used in predicate without comparison fails on sqlite (and other non-sql server backends?) #30666
Conversation
There was a problem hiding this comment.
It seems like we already deal with this for non-JSON cases in RelationalValueConverterCompensatingExpressionVisitor - is there simply a bug in that visitor when the expression happens to be JSON? In any case, it doesn't seem like we should have two places dealing with the bool+value converter case, if possible...
Unrelated General question... Is there a reason we need to run SearchConditionConvertingExpressionVisitor in RelationalParameterBasedSqlProcessor, rather than in RelationalQueryTranslationPostprocessor? It doesn't seem to need to look at any parameter values, is there some ordering dependency where it must run after e.g. the nullability processor? I'm asking since this PR now introduces this additional visitation for all providers (not just SQL Server), and the parameter-based processor gets invoked again for different parameter nullabilities etc. (not critical and can be done separately)
src/EFCore.Relational/Query/SearchConditionConvertingExpressionVisitor.cs
Outdated
Show resolved
Hide resolved
src/EFCore.Relational/Query/RelationalParameterBasedSqlProcessor.cs
Outdated
Show resolved
Hide resolved
src/EFCore.Relational/Query/SearchConditionConvertingExpressionVisitor.cs
Outdated
Show resolved
Hide resolved
src/EFCore.Relational/Query/SearchConditionConvertingExpressionVisitor.cs
Outdated
Show resolved
Hide resolved
src/EFCore.SqlServer/Query/Internal/SkipTakeCollapsingExpressionVisitor.cs
Outdated
Show resolved
Hide resolved
src/EFCore.Sqlite.Core/Query/Internal/SqliteParameterBasedSqlProcessor.cs
Outdated
Show resolved
Hide resolved
|
@roji updated. Much better and simpler change. Thanks for the hint! |
src/EFCore.Relational/Query/Internal/RelationalValueConverterCompensatingExpressionVisitor.cs
Outdated
Show resolved
Hide resolved
maumar
force-pushed
the
fix30326
branch
2 times, most recently
from
0e6be52
to
e5edcb8
Compare
…comparison fails on sqlite (and other non-sql server backends?) RelationalValueConverterCompensatingExpressionVisitor was not compensating for bool values with converters for JsonScalarExpression. Fixes #30326
SqlServer doesn't have the problem because search condition replacing visitor already converts all bool values in the predicate into conditions. Fix is to re-purpose this visitor to be used by other providers also and convert bool values to conditions when they have a converter.
Fixes #30326