Implement `IS [NOT] DISTINCT FROM` translation
Most SQL engines support the IS [NOT] DISTINCT FROM predicate, which conveniently matches C# semantics for !=/==.
Where available (and efficient aka supported by indexes), using it can lead to simpler queries.
Fixes #10514, fixes #29624.
As mentioned in https://github.com/dotnet/efcore/issues/29624#issuecomment-2182047136 this is still WIP (opened as PR for viewing convenience @roji ).
Will require Compat level 160
I will work more on this in the weekend.
⚠️ This changeset might cause is a significant reduction in the test coverage for RewriteNullSemantics.
Will think about this in the weekend (maybe we can explicitly test for the interesting cases with a group of tests that intentionally "downgrade" the max sqlserver version).
I added the checks for SQLite version >= 3.8.11 and SqlServer CompatibilityVersion >= 160
Several tests are failing on SqlServer:
- Compare_negated_bool_with_negated_bool_equal
- Compare_bool_with_negated_bool_not_equal
- Compare_negated_bool_with_bool_not_equal
- Compare_negated_bool_with_bool_equal_negated
- Compare_negated_bool_with_negated_bool_not_equal
- Compare_negated_bool_with_bool_equal
- Compare_bool_with_negated_bool_equal
- Compare_bool_with_negated_bool_equal_negated
- Compare_bool_with_negated_bool_not_equal_negated
- Compare_negated_bool_with_negated_bool_equal_negated
- Compare_negated_bool_with_bool_not_equal_negated
- Compare_negated_bool_with_negated_bool_not_equal_negated
IIUC, this is because of https://github.com/dotnet/efcore/issues/34001
Note that currently the test have not yet been extended to check both the IS and the relational-null-replacement code paths.
I pushed a commit that avoids using IS [NOT] DISTINCT FROM when that would trigger #34001.
Obviously it would still be desirable to (also) fix #34001
As an intermediate step, I opened #34072 which adds exhaustive testing of the null rewriting (plus some cleanup/minor improvements ;) ).
My current plan would be to run this test for SQL Server both normally and forcing a lower compatibility version for these tests (following the model of PrimitiveCollectionsQueryOldSqlServerTest).
I rebased this PR to resolve conflicts and to start getting to the issues that have been reported:
- [x] rename
IsExpressiontoIsNotDistinctFromExpressionorIsDistinctFromExpression; I would like to keep the equality case as base, but I am afraid thatIsNotDistinctFromExpressionis complex (IsDistinctFromExpressionhas the disadvantage that it is the not-equal) - [x] add tests to ensure that there are no regressions in the null rewrite added in #34072
- [x] check precedence of
IS DISTINCT FROMin SqlServer - [x] remove bad translations of nullable
bool?comparisons in SqlServer https://github.com/dotnet/efcore/pull/34048#discussion_r1650812620 - [x] improve comparisons of nullable
bool?by taking advantage of https://github.com/dotnet/efcore/pull/34080 (optional, desirable) - [ ] get the PR to pass the tests in CI
- [x] introduce a reusable abstraction for providers (
ApplyEqualityNullSemantics; might need some more polishing) - [ ] add an
AppContextswitch to opt out of the translation
I would like to keep the equality case as base, but I am afraid that IsNotDistinctFromExpression is complex (IsDistinctFromExpression has the disadvantage that it is the not-equal)
Yeah, the negated logic here is unfortunate. There's also the option of having two distinct expression types, though that would make it more complicated to handled in visitors needing to identify them.
Rebased to solve conflicts and keep an eye on SQL Server updates in CI 😉
This is currently on top of https://github.com/dotnet/efcore/pull/34166 which affects the translation of boolean comparisons