Additional perf improvements around null semantics scenarios #18694

maumar · 2019-11-01T02:56:18Z

Resolves #17543 - Queries really slow due to null checks
Resolves #18525 - Query: optimize binary expression AndAlso and OrElse where left and right are the same
Resolves #18547 - DbFunctions compared to NULL are ignored and break the query builder

#17543

Before when rewriting null comparisons we would always remove possibility of nulls in the resulting expression. This is not always needed, specifically in predicates, where it doesn't really matter if expression returns false or null - the result is the same.
Now we detect those cases and apply "simplified" null expansion which should lead to better performance.

#18525

Null semantics initially expands expressions to a verbose form which then gets simplified in query optimizer. One of the optimizations added is when we do AND/OR where both sides are the same. Other small improvements have been added in this change also.

#18547

Previously we assumed that function can only be null if at least one of its arguments is null. This is the case for most built-in functions, but not all. This also goes for user defined functions which can return arbitrary results.
Fix is to treat all functions as potentially nullable. This leads to worse queries is some cases, but is also mitigated by other improvements added along side.
In the future we will provide metadata to better determine given function's nullability.

Also fixed a few small bug found along the way - incorrect Equals comparison for SqlExpression, for cases when TypeMapping was null.

maumar · 2019-11-01T02:57:58Z

cc: @ajcvickers @Pilchie

maumar · 2019-11-01T03:00:52Z

test/EFCore.SqlServer.FunctionalTests/Query/FunkyDataQuerySqlServerTest.cs

@@ -275,7 +275,7 @@ public override async Task String_starts_with_on_argument_with_wildcard_column_n
                @"SELECT [f].[FirstName] AS [fn], [f0].[LastName] AS [ln]
 FROM [FunkyCustomers] AS [f]
 CROSS JOIN [FunkyCustomers] AS [f0]
-WHERE (([f0].[LastName] <> N'') OR [f0].[LastName] IS NULL) AND ([f].[FirstName] IS NOT NULL AND ([f0].[LastName] IS NOT NULL AND (LEFT([f].[FirstName], LEN([f0].[LastName])) <> [f0].[LastName])))");
+WHERE (([f0].[LastName] <> N'') OR [f0].[LastName] IS NULL) AND ([f].[FirstName] IS NOT NULL AND ([f0].[LastName] IS NOT NULL AND ((LEFT([f].[FirstName], LEN([f0].[LastName])) <> [f0].[LastName]) OR LEFT([f].[FirstName], LEN([f0].[LastName])) IS NULL)))");


sql is worse due to #18547, we have issue tracking improvement for those here: #18555

smitpatel · 2019-11-01T21:30:04Z

Commit message has wrong issue reference.

src/EFCore.SqlServer/Query/Internal/SqlServerQueryTranslationPostprocessor.cs

src/EFCore.Relational/Query/Internal/SqlExpressionOptimizingExpressionVisitor.cs

src/EFCore.Relational/Query/Internal/NullSemanticsRewritingExpressionVisitor.cs

maumar · 2019-11-01T23:21:13Z

@smitpatel new version up

Resolves #17543 - Queries really slow due to null checks Resolves #18525 - Query: optimize binary expression AndAlso and OrElse where left and right are the same Resolves #18547 - DbFunctions compared to NULL are ignored and break the query builder #17543 Before when rewriting null comparisons we would always remove possibility of nulls in the resulting expression. This is not always needed, specifically in predicates, where it doesn't really matter if expression returns false or null - the result is the same. Now we detect those cases and apply "simplified" null expansion which should lead to better performance. #18525 Null semantics initially expands expressions to a verbose form which then gets simplified in query optimizer. One of the optimizations added is when we do AND/OR where both sides are the same. Other small improvements have been added in this change also. #18547 Previously we assumed that function can only be null if at least one of its arguments is null. This is the case for most built-in functions, but not all. This also goes for user defined functions which can return arbitrary results. Fix is to treat all functions as potentially nullable. This leads to worse queries is some cases, but is also mitigated by other improvements added along side. In the future we will provide metadata to better determine given function's nullability. Also fixed a few small bug found along the way - incorrect Equals comparison for SqlExpression, for cases when TypeMapping was null. Additional refactoring: - removed the second pass of sql optimizations (we do it later when sniffing parameter values anyway) - consolidated NullComparisonTransformingExpressionVisitor into SqlExpressionOptimizingExpressionVisitor

src/EFCore.Relational/Query/Internal/NullSemanticsRewritingExpressionVisitor.cs

AndriySvyryd · 2019-11-01T23:43:48Z

src/EFCore.Relational/Query/Internal/NullSemanticsRewritingExpressionVisitor.cs

+            {
+                _canOptimize = testIsCondition;
+                var newTest = (SqlExpression)Visit(whenClause.Test);
+                _canOptimize = false;


There's no need to keep setting it to false. There's no code that could set it to true

AndriySvyryd · 2019-11-01T23:50:03Z

src/EFCore.Relational/Query/SqlExpressions/SqlExpression.cs

@@ -32,7 +32,8 @@ public override bool Equals(object obj)

        private bool Equals(SqlExpression sqlExpression)
            => Type == sqlExpression.Type
-                && TypeMapping?.Equals(sqlExpression.TypeMapping) == true;
+               && ((TypeMapping == null && sqlExpression.TypeMapping == null)
+                || TypeMapping?.Equals(sqlExpression.TypeMapping) == true);


Equals(TypeMapping, sqlExpression.TypeMapping)

maumar requested review from smitpatel, AndriySvyryd and roji November 1, 2019 02:56

maumar commented Nov 1, 2019

View reviewed changes

maumar force-pushed the fix17543 branch from 5feb423 to 270e0c4 Compare November 1, 2019 18:38