Use null propagation to optimize away `IS NOT NULL` checks #34127

ranma42 · 2024-06-30T19:42:24Z

When a CASE expression simply replicates SQL null propagation, simplify it.

Contributes to #34126.

src/EFCore.Relational/Query/SqlNullabilityProcessor.cs

roji

Great work as always @ranma42, it's really nice to see our SQL baselines getting progressively tighter and better.

See mainly nits below.

roji · 2024-07-12T10:58:01Z

src/EFCore.Relational/Query/SqlNullabilityProcessor.cs

+        }
+
+        // optimize expressions such as expr != null ? expr : null and expr == null ? null : expr
+        if (testIsCondition && whenClauses is [var clause] && (elseResult == null || IsNull(clause.Result)))


I'm personally moving to is null pretty systematically (here and below):

Suggested change

if (testIsCondition && whenClauses is [var clause] && (elseResult == null || IsNull(clause.Result)))

if (testIsCondition && whenClauses is [var clause] && (elseResult is null || IsNull(clause.Result)))

(I actually use is for any constant/literal check at this point).

roji · 2024-07-12T11:01:21Z

src/EFCore.Relational/Query/SqlNullabilityProcessor.cs

+            SqlExpression test, expr;
+
+            if (elseResult == null)
+            {
+                expr = clause.Result;
+                test = clause.Test;
+            }
+            else
+            {
+                expr = elseResult;
+                test = _sqlExpressionFactory.Not(clause.Test);
+            }


nit:

Suggested change

SqlExpression test, expr;

if (elseResult == null)

{

expr = clause.Result;

test = clause.Test;

}

else

{

expr = elseResult;

test = _sqlExpressionFactory.Not(clause.Test);

}

var (test, expr) = elseResult is null

? (clause.Result, clause.Test)

: (_sqlExpressionFactory.Not(clause.Test), elseResult);

... and move nullPropagatedOperands below.

oops, this was swapped (clause.Result, clause.Test) vs (clause.Test, clause.Result)

roji · 2024-07-12T11:05:15Z

src/EFCore.Relational/Query/SqlNullabilityProcessor.cs

+        {
+            operands.Add(expression);
+
+            if (expression is SqlUnaryExpression unary


nit: this can all be one nice switch statement with pattern matching ;)

will it work even if the result is void? I am unable to get its syntax right :\

uh, statement, not expression... right!

roji · 2024-07-12T11:09:31Z

src/EFCore.Relational/Query/SqlNullabilityProcessor.cs

+            };
+
+        // FIXME: unify nullability computations
+        static void NullPropagatedOperands(SqlExpression expression, HashSet<SqlExpression> operands)


Naming-wise, maybe DetectNullPropagatingNodes?

roji · 2024-07-12T11:12:24Z

src/EFCore.Relational/Query/SqlNullabilityProcessor.cs

+                _ => expression,
+            };
+
+        // FIXME: unify nullability computations


Yeah, this definitely feels like something more general than as a very local utility to case, plus there should be extensibility for provider-specific expression types. We could even compute this information and bubble it up as part of the normal visitation of SqlNullabilityProcessor - assuming it's useful enough for other parts of the visitor.

But of course this can all be done later.

This is what I was considering doing in #33889 (comment) (instead of simple boolean nullability, propagate the "nullability expression") 😉
I will most likely do that in a later experiment, at the very least to evaluate whether the nullability expressions are simple to create/consume.

Yep, makes sense!

roji · 2024-07-12T11:13:53Z

src/EFCore.Relational/Query/SqlNullabilityProcessor.cs

+                NullPropagatedOperands(binary.Left, operands);
+                NullPropagatedOperands(binary.Right, operands);
+            }
+            else if (expression is SqlFunctionExpression { IsNullable: true } func)


AtTimeZone, Collate (which are types of binary expressions)

this function is mostly based on ProcessNullNotNull (which, just like this function, ignores AtTimeZone, Collate, ...)

I see... We should probably fix that in both places - if you prefer not to do this in this PR we can open a separate issue.

I will add the part specific to this PR here; I added the other part in #34263

added in 12a532b

src/EFCore.Relational/Query/SqlNullabilityProcessor.cs

roji · 2024-07-12T11:35:40Z

src/EFCore.Relational/Query/SqlNullabilityProcessor.cs

+            operands.Add(expression);
+
+            if (expression is SqlUnaryExpression unary
+                && unary.OperatorType is ExpressionType.Not or ExpressionType.Negate or ExpressionType.Convert)


Are there other unary operators that propagate? Maybe better to exclude those that you don't want, rather than specifying those that you do?

the advantage of specifying them in a positive way is that changing the supported OperatorTypes is safe
OTOH it is unlikely to happen and would likely require other changes across the repo, so I can replace this by excluding Equal and NotEqual if that is easier to read

Just noting that binary just below is negative rather than positive. Up to you - the possible values here in ExpressionType is a closed set, so it should be safe either way.

src/EFCore.Relational/Query/SqlNullabilityProcessor.cs

When a CASE expression simply replicates SQL null propagation, simplify it.

as per review.

ranma42 changed the title ~~Optimize null propagation~~ Use null propagation to optimize away IS NOT NULL checks Jun 30, 2024

ranma42 force-pushed the drop-not-null-checks branch 2 times, most recently from 3769918 to afcfba2 Compare June 30, 2024 19:46

ranma42 commented Jul 1, 2024

View reviewed changes

src/EFCore.Relational/Query/SqlNullabilityProcessor.cs Outdated Show resolved Hide resolved

ranma42 marked this pull request as draft July 2, 2024 23:39

ranma42 force-pushed the drop-not-null-checks branch from afcfba2 to d9c0736 Compare July 3, 2024 15:54

ranma42 marked this pull request as ready for review July 3, 2024 15:54

ranma42 marked this pull request as draft July 3, 2024 15:56

ranma42 force-pushed the drop-not-null-checks branch 2 times, most recently from 089d329 to fb551f8 Compare July 3, 2024 18:42

ranma42 marked this pull request as ready for review July 4, 2024 07:11

ranma42 mentioned this pull request Jul 4, 2024

Consider introducing nullability on SqlExpression #33889

Open

ranma42 force-pushed the drop-not-null-checks branch from fb551f8 to bbe125a Compare July 9, 2024 22:52

roji self-assigned this Jul 12, 2024

roji requested changes Jul 12, 2024

View reviewed changes

ranma42 force-pushed the drop-not-null-checks branch from c2b1eff to 69ae99a Compare July 12, 2024 16:40

ranma42 added 4 commits July 13, 2024 00:48

Add tests for null check removal

8aa7e2d

Use null propagation to optimize away IS NOT NULL checks

bb31c27

When a CASE expression simply replicates SQL null propagation, simplify it.

Update baselines

0695eb4

Implement improvements from code review

65d9a68

ranma42 force-pushed the drop-not-null-checks branch from 69ae99a to 65d9a68 Compare July 12, 2024 22:49

ranma42 mentioned this pull request Jul 21, 2024

Implement nullability simplification for COLLATE and AT TIME ZONE #34263

Merged

Handle AtTimeZoneExpression and CollateExpression

12a532b

as per review.

ranma42 requested a review from roji July 21, 2024 15:44

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Use null propagation to optimize away `IS NOT NULL` checks #34127

Use null propagation to optimize away `IS NOT NULL` checks #34127

ranma42 commented Jun 30, 2024

roji left a comment

roji Jul 12, 2024

roji Jul 12, 2024

ranma42 Jul 12, 2024

roji Jul 12, 2024

ranma42 Jul 12, 2024

ranma42 Jul 12, 2024

roji Jul 12, 2024

roji Jul 12, 2024

ranma42 Jul 12, 2024

roji Jul 12, 2024

roji Jul 12, 2024

ranma42 Jul 12, 2024

roji Jul 12, 2024 •

edited

Loading

ranma42 Jul 21, 2024

ranma42 Jul 27, 2024

roji Jul 12, 2024

ranma42 Jul 12, 2024 •

edited

Loading

roji Jul 12, 2024

	if (testIsCondition && whenClauses is [var clause] && (elseResult == null \|\| IsNull(clause.Result)))
	if (testIsCondition && whenClauses is [var clause] && (elseResult is null \|\| IsNull(clause.Result)))

Use null propagation to optimize away IS NOT NULL checks #34127

Are you sure you want to change the base?

Use null propagation to optimize away IS NOT NULL checks #34127

Conversation

ranma42 commented Jun 30, 2024

roji left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

roji Jul 12, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ranma42 Jul 12, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Use null propagation to optimize away `IS NOT NULL` checks #34127

Use null propagation to optimize away `IS NOT NULL` checks #34127

roji Jul 12, 2024 •

edited

Loading

ranma42 Jul 12, 2024 •

edited

Loading