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

Simplify and improve null semantics rewrites #34072

Merged
merged 6 commits into from
Jun 27, 2024
Merged

Conversation

ranma42
Copy link
Contributor

@ranma42 ranma42 commented Jun 23, 2024

This PR contains:

  • exhaustive testing of null rewrites
  • a simpler implementation of the rewrite, which instead of hand-coding all of the cases, combines the generic case and the already available simplifications
  • a more generic translation of the rewrite optimized for predicates, which simplifies some existing tests.

@ranma42
Copy link
Contributor Author

ranma42 commented Jun 23, 2024

These changes stemmed from #34048, in particular the testing part.

@ranma42
Copy link
Contributor Author

ranma42 commented Jun 24, 2024

Note that the translation is (mostly) the same as before:

x == y

in C# translates to the SQL expression

(x = y AND x IS NOT NULL AND y IS NOT NULL) OR (x IS NULL AND y IS NULL)

other translations are possible, such as:

CASE
  WHEN x = y OR (x IS NULL AND y IS NULL) THEN TRUE
  ELSE FALSE
END

(or COALESCE(x = y OR (x IS NULL AND y IS NULL), FALSE), which is essentially the same). This is basically using the translation optimized for predicates and covering the NULL cases with ELSE or COALESCE. This might further cleanup the SqlServer translations such as:

SELECT [e].[Id], CASE
    WHEN [e].[NullableIntA] = [e].[IntB] AND [e].[NullableIntA] IS NOT NULL THEN CAST(1 AS bit)
    ELSE CAST(0 AS bit)
END AS [X]
FROM [Entities1] AS [e]

but maybe it is better to tackle this in other ways (for example changing how the pipeline runs, so that the SearchConditionConvertingExpressionVisitor can inform the NullProcessor that an expression is within a predicate).

@maumar
Copy link
Contributor

maumar commented Jun 27, 2024

now that is one hell of a simplification! 🎉 RIP truth tables (ຈ ﹏ ຈ )

@maumar maumar merged commit 1847e1e into dotnet:main Jun 27, 2024
7 checks passed
@maumar
Copy link
Contributor

maumar commented Jun 27, 2024

AWESOME stuff, thanks a lot @ranma42 !

@ranma42
Copy link
Contributor Author

ranma42 commented Jun 27, 2024

now that is one hell of a simplification! 🎉 RIP truth tables (ຈ ﹏ ຈ )

They are useful, but as much as possible I would like to leave that approach to machines (they are much better than people at exhaustively checking that all of the cases match the expectations 😉 )

@ranma42 ranma42 deleted the null-rewriting branch June 27, 2024 05:53
roji pushed a commit to roji/efcore that referenced this pull request Jun 27, 2024
- exhaustive testing of null rewrites
- a simpler implementation of the rewrite, which instead of hand-coding all of the cases, combines the generic case and the already available simplifications
- a more generic translation of the rewrite optimized for predicates, which simplifies some existing tests.
roji pushed a commit to roji/efcore that referenced this pull request Jun 28, 2024
- exhaustive testing of null rewrites
- a simpler implementation of the rewrite, which instead of hand-coding all of the cases, combines the generic case and the already available simplifications
- a more generic translation of the rewrite optimized for predicates, which simplifies some existing tests.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants