From 423cc582bb986270b7d478635e495acddfe4cf40 Mon Sep 17 00:00:00 2001 From: Shay Rojansky Date: Sat, 20 Nov 2021 18:54:22 +0100 Subject: [PATCH] Pattern-match to PG row value comparison (#2112) Closes #2111 --- EFCore.PG.sln.DotSettings | 1 + .../NpgsqlServiceCollectionExtensions.cs | 1 + .../Internal/PostgresRowValueExpression.cs | 134 ++++++++++++ .../NpgsqlPostgresSqlOptimizingVisitor.cs | 143 +++++++++++++ .../Query/Internal/NpgsqlQuerySqlGenerator.cs | 22 ++ .../NpgsqlQueryTranslationPostprocessor.cs | 44 ++++ ...sqlQueryTranslationPostprocessorFactory.cs | 45 ++++ .../Internal/NpgsqlSqlNullabilityProcessor.cs | 104 ++------- .../NpgsqlSqlTranslatingExpressionVisitor.cs | 12 +- .../Query/PostgresOptimizationsQueryTest.cs | 200 ++++++++++++++++++ 10 files changed, 624 insertions(+), 82 deletions(-) create mode 100644 src/EFCore.PG/Query/Expressions/Internal/PostgresRowValueExpression.cs create mode 100644 src/EFCore.PG/Query/Internal/NpgsqlPostgresSqlOptimizingVisitor.cs create mode 100644 src/EFCore.PG/Query/Internal/NpgsqlQueryTranslationPostprocessor.cs create mode 100644 src/EFCore.PG/Query/Internal/NpgsqlQueryTranslationPostprocessorFactory.cs create mode 100644 test/EFCore.PG.FunctionalTests/Query/PostgresOptimizationsQueryTest.cs diff --git a/EFCore.PG.sln.DotSettings b/EFCore.PG.sln.DotSettings index 4fa3552d..40864433 100644 --- a/EFCore.PG.sln.DotSettings +++ b/EFCore.PG.sln.DotSettings @@ -228,6 +228,7 @@ True True True + True True True True diff --git a/src/EFCore.PG/Extensions/NpgsqlServiceCollectionExtensions.cs b/src/EFCore.PG/Extensions/NpgsqlServiceCollectionExtensions.cs index 2578f2f6..500a0e81 100644 --- a/src/EFCore.PG/Extensions/NpgsqlServiceCollectionExtensions.cs +++ b/src/EFCore.PG/Extensions/NpgsqlServiceCollectionExtensions.cs @@ -129,6 +129,7 @@ public static IServiceCollection AddEntityFrameworkNpgsql(this IServiceCollectio .TryAdd(p => p.GetRequiredService()) .TryAdd() .TryAdd() + .TryAdd() .TryAddProviderSpecificServices( b => b .TryAddSingleton() diff --git a/src/EFCore.PG/Query/Expressions/Internal/PostgresRowValueExpression.cs b/src/EFCore.PG/Query/Expressions/Internal/PostgresRowValueExpression.cs new file mode 100644 index 00000000..215356db --- /dev/null +++ b/src/EFCore.PG/Query/Expressions/Internal/PostgresRowValueExpression.cs @@ -0,0 +1,134 @@ +using System; +using System.Collections.Generic; +using System.Linq.Expressions; +using Microsoft.EntityFrameworkCore.Query; +using Microsoft.EntityFrameworkCore.Query.SqlExpressions; +using Microsoft.EntityFrameworkCore.Utilities; + +namespace Npgsql.EntityFrameworkCore.PostgreSQL.Query.Expressions.Internal +{ + /// + /// An expression that represents a PostgreSQL-specific row value expression in a SQL tree. + /// + /// + /// See the PostgreSQL docs + /// for more information. + /// + public class PostgresRowValueExpression : SqlExpression, IEquatable + { + /// + /// The operator of this PostgreSQL binary operation. + /// + public virtual IReadOnlyList RowValues { get; } + + /// + public PostgresRowValueExpression(IReadOnlyList rowValues) + : base(typeof(Array), typeMapping: null) + { + Check.NotNull(rowValues, nameof(rowValues)); + + RowValues = rowValues; + } + + public virtual PostgresRowValueExpression Prepend(SqlExpression expression) + { + var newRowValues = new SqlExpression[RowValues.Count + 1]; + newRowValues[0] = expression; + for (var i = 1; i < newRowValues.Length; i++) + { + newRowValues[i] = RowValues[i - 1]; + } + + return new PostgresRowValueExpression(newRowValues); + } + + /// + protected override Expression VisitChildren(ExpressionVisitor visitor) + { + Check.NotNull(visitor, nameof(visitor)); + + SqlExpression[]? newRowValues = null; + + for (var i = 0; i < RowValues.Count; i++) + { + var rowValue = RowValues[i]; + var visited = (SqlExpression)visitor.Visit(rowValue); + if (visited != rowValue && newRowValues is null) + { + newRowValues = new SqlExpression[RowValues.Count]; + for (var j = 0; j < i; i++) + { + newRowValues[j] = RowValues[j]; + } + } + + if (newRowValues is not null) + { + newRowValues[i] = visited; + } + } + + return newRowValues is null ? this : new PostgresRowValueExpression(newRowValues); + } + + /// + protected override void Print(ExpressionPrinter expressionPrinter) + { + expressionPrinter.Append("("); + + var count = RowValues.Count; + for (var i = 0; i < count; i++) + { + expressionPrinter.Visit(RowValues[i]); + + if (i < count - 1) + { + expressionPrinter.Append(", "); + } + } + + expressionPrinter.Append(")"); + } + + /// + public override bool Equals(object? obj) + => obj is PostgresRowValueExpression other && Equals(other); + + /// + public virtual bool Equals(PostgresRowValueExpression? other) + { + if (other is null || !base.Equals(other) || other.RowValues.Count != RowValues.Count) + { + return false; + } + + if (ReferenceEquals(this, other)) + { + return true; + } + + for (var i = 0; i < RowValues.Count; i++) + { + if (other.RowValues[i].Equals(RowValues[i])) + { + return false; + } + } + + return true; + } + + /// + public override int GetHashCode() + { + var hashCode = new HashCode(); + + foreach (var rowValue in RowValues) + { + hashCode.Add(rowValue); + } + + return hashCode.ToHashCode(); + } + } +} \ No newline at end of file diff --git a/src/EFCore.PG/Query/Internal/NpgsqlPostgresSqlOptimizingVisitor.cs b/src/EFCore.PG/Query/Internal/NpgsqlPostgresSqlOptimizingVisitor.cs new file mode 100644 index 00000000..3444982d --- /dev/null +++ b/src/EFCore.PG/Query/Internal/NpgsqlPostgresSqlOptimizingVisitor.cs @@ -0,0 +1,143 @@ +using System.Diagnostics; +using System.Diagnostics.CodeAnalysis; +using System.Linq.Expressions; +using Microsoft.EntityFrameworkCore.Query; +using Microsoft.EntityFrameworkCore.Query.SqlExpressions; +using Npgsql.EntityFrameworkCore.PostgreSQL.Query.Expressions.Internal; + +namespace Npgsql.EntityFrameworkCore.PostgreSQL.Query.Internal +{ + /// + /// Performs various PostgreSQL-specific optimizations to the SQL expression tree. + /// + public class NpgsqlPostgresSqlOptimizingVisitor : ExpressionVisitor + { + private readonly ISqlExpressionFactory _sqlExpressionFactory; + + public NpgsqlPostgresSqlOptimizingVisitor(ISqlExpressionFactory sqlExpressionFactory) + => _sqlExpressionFactory = sqlExpressionFactory; + + [return: NotNullIfNotNull("expression")] + public override Expression? Visit(Expression? expression) + { + switch (expression) + { + case ShapedQueryExpression shapedQueryExpression: + return shapedQueryExpression.Update( + Visit(shapedQueryExpression.QueryExpression), + shapedQueryExpression.ShaperExpression); + + case SqlBinaryExpression { OperatorType: ExpressionType.OrElse } sqlBinaryExpression: + { + var visited = (SqlBinaryExpression)base.Visit(sqlBinaryExpression); + + var x = TryConvertToRowValueComparison(visited, out var rowComparisonExpression) + ? rowComparisonExpression + : visited; + + return x; + } + + default: + return base.Visit(expression); + } + } + + private bool TryConvertToRowValueComparison( + SqlBinaryExpression sqlOrExpression, + [NotNullWhen(true)] out SqlBinaryExpression? rowComparisonExpression) + { + // This pattern matches x > 5 || x == 5 && y > 6, and converts it to (x, y) > (5, 6) + Debug.Assert(sqlOrExpression.OperatorType == ExpressionType.OrElse); + + if (TryMatchTopExpression(sqlOrExpression.Left, sqlOrExpression.Right, out rowComparisonExpression) + || TryMatchTopExpression(sqlOrExpression.Right, sqlOrExpression.Left, out rowComparisonExpression)) + { + return true; + } + + rowComparisonExpression = null; + return false; + + bool TryMatchTopExpression( + SqlExpression first, + SqlExpression second, + [NotNullWhen(true)] out SqlBinaryExpression? rowComparisonExpression) + { + if (first is SqlBinaryExpression { OperatorType: ExpressionType.AndAlso } subExpression + && second is SqlBinaryExpression comparisonExpression1 + && IsComparisonOperator(comparisonExpression1.OperatorType)) + { + if (TryMatchBottomExpression(subExpression.Left, subExpression.Right, out var equalityExpression, out var comparisonExpression2) + || TryMatchBottomExpression(subExpression.Right, subExpression.Left, out equalityExpression, out comparisonExpression2)) + { + // We've found a structural match. Now make sure the operands and operators correspond + if (comparisonExpression1.Left.Equals(equalityExpression.Left) + && comparisonExpression1.Right.Equals(equalityExpression.Right) + && comparisonExpression1.OperatorType == comparisonExpression2.OperatorType) + { + // Bingo. + + // If we're composing over an existing row value comparison, just prepend the new element to it. + // Otherwise create a new row value comparison expression. + if (comparisonExpression2.Left is PostgresRowValueExpression leftRowValueExpression) + { + var rightRowValueExpression = (PostgresRowValueExpression)comparisonExpression2.Right; + + rowComparisonExpression = _sqlExpressionFactory.MakeBinary( + comparisonExpression1.OperatorType, + leftRowValueExpression.Prepend(comparisonExpression1.Left), + rightRowValueExpression.Prepend(comparisonExpression1.Right), + typeMapping: null)!; + } + else + { + rowComparisonExpression = _sqlExpressionFactory.MakeBinary( + comparisonExpression1.OperatorType, + new PostgresRowValueExpression(new[] { comparisonExpression1.Left, comparisonExpression2.Left }), + new PostgresRowValueExpression(new[] { comparisonExpression1.Right, comparisonExpression2.Right }), + typeMapping: null)!; + } + + return true; + } + } + } + + rowComparisonExpression = null; + return false; + + static bool TryMatchBottomExpression( + SqlExpression first, + SqlExpression second, + [NotNullWhen(true)] out SqlBinaryExpression? equalityExpression, + [NotNullWhen(true)] out SqlBinaryExpression? comparisonExpression) + { + // This pattern matches the bottom expression of the pattern: x == 5 && y > 6 + + if (first is SqlBinaryExpression { OperatorType: ExpressionType.Equal } equalityExpression2 + && second is SqlBinaryExpression comparisonExpression2 + && IsComparisonOperator(comparisonExpression2.OperatorType)) + { + equalityExpression = equalityExpression2; + comparisonExpression = comparisonExpression2; + return true; + } + + equalityExpression = comparisonExpression = null; + return false; + } + } + } + + private static bool IsComparisonOperator(ExpressionType expressionType) + => expressionType switch + { + ExpressionType.GreaterThan => true, + ExpressionType.GreaterThanOrEqual => true, + ExpressionType.LessThan => true, + ExpressionType.LessThanOrEqual => true, + _ => false + }; + } +} diff --git a/src/EFCore.PG/Query/Internal/NpgsqlQuerySqlGenerator.cs b/src/EFCore.PG/Query/Internal/NpgsqlQuerySqlGenerator.cs index 90382b74..8a9ac42d 100644 --- a/src/EFCore.PG/Query/Internal/NpgsqlQuerySqlGenerator.cs +++ b/src/EFCore.PG/Query/Internal/NpgsqlQuerySqlGenerator.cs @@ -58,6 +58,7 @@ protected override Expression VisitExtension(Expression extensionExpression) PostgresJsonTraversalExpression jsonTraversalExpression => VisitJsonPathTraversal(jsonTraversalExpression), PostgresNewArrayExpression newArrayExpression => VisitPostgresNewArray(newArrayExpression), PostgresRegexMatchExpression regexMatchExpression => VisitRegexMatch(regexMatchExpression), + PostgresRowValueExpression rowValueExpression => VisitRowValue(rowValueExpression), PostgresUnknownBinaryExpression unknownBinaryExpression => VisitUnknownBinary(unknownBinaryExpression), _ => base.VisitExtension(extensionExpression) }; @@ -556,6 +557,27 @@ public virtual Expression VisitRegexMatch(PostgresRegexMatchExpression expressio return expression; } + public virtual Expression VisitRowValue(PostgresRowValueExpression rowValueExpression) + { + Sql.Append("("); + + var values = rowValueExpression.RowValues; + var count = values.Count; + for (var i = 0; i < count; i++) + { + Visit(values[i]); + + if (i < count - 1) + { + Sql.Append(", "); + } + } + + Sql.Append(")"); + + return rowValueExpression; + } + /// /// Visits the children of an . /// diff --git a/src/EFCore.PG/Query/Internal/NpgsqlQueryTranslationPostprocessor.cs b/src/EFCore.PG/Query/Internal/NpgsqlQueryTranslationPostprocessor.cs new file mode 100644 index 00000000..c0e60c3d --- /dev/null +++ b/src/EFCore.PG/Query/Internal/NpgsqlQueryTranslationPostprocessor.cs @@ -0,0 +1,44 @@ +using System.Linq.Expressions; +using Microsoft.EntityFrameworkCore.Query; + +namespace Npgsql.EntityFrameworkCore.PostgreSQL.Query.Internal +{ + /// + /// This is an internal API that supports the Entity Framework Core infrastructure and not subject to + /// the same compatibility standards as public APIs. It may be changed or removed without notice in + /// any release. You should only use it directly in your code with extreme caution and knowing that + /// doing so can result in application failures when updating to a new Entity Framework Core release. + /// + public class NpgsqlQueryTranslationPostprocessor : RelationalQueryTranslationPostprocessor + { + private readonly NpgsqlPostgresSqlOptimizingVisitor _npgsqlPostgresSqlOptimizingVisitor; + + /// + /// This is an internal API that supports the Entity Framework Core infrastructure and not subject to + /// the same compatibility standards as public APIs. It may be changed or removed without notice in + /// any release. You should only use it directly in your code with extreme caution and knowing that + /// doing so can result in application failures when updating to a new Entity Framework Core release. + /// + public NpgsqlQueryTranslationPostprocessor( + QueryTranslationPostprocessorDependencies dependencies, + RelationalQueryTranslationPostprocessorDependencies relationalDependencies, + QueryCompilationContext queryCompilationContext) + : base(dependencies, relationalDependencies, queryCompilationContext) + => _npgsqlPostgresSqlOptimizingVisitor = new(relationalDependencies.SqlExpressionFactory); + + /// + /// This is an internal API that supports the Entity Framework Core infrastructure and not subject to + /// the same compatibility standards as public APIs. It may be changed or removed without notice in + /// any release. You should only use it directly in your code with extreme caution and knowing that + /// doing so can result in application failures when updating to a new Entity Framework Core release. + /// + public override Expression Process(Expression query) + { + query = _npgsqlPostgresSqlOptimizingVisitor.Visit(query); + + query = base.Process(query); + + return query; + } + } +} diff --git a/src/EFCore.PG/Query/Internal/NpgsqlQueryTranslationPostprocessorFactory.cs b/src/EFCore.PG/Query/Internal/NpgsqlQueryTranslationPostprocessorFactory.cs new file mode 100644 index 00000000..c65d8871 --- /dev/null +++ b/src/EFCore.PG/Query/Internal/NpgsqlQueryTranslationPostprocessorFactory.cs @@ -0,0 +1,45 @@ +using Microsoft.EntityFrameworkCore.Query; + +namespace Npgsql.EntityFrameworkCore.PostgreSQL.Query.Internal +{ + public class NpgsqlQueryTranslationPostprocessorFactory : IQueryTranslationPostprocessorFactory + { + /// + /// This is an internal API that supports the Entity Framework Core infrastructure and not subject to + /// the same compatibility standards as public APIs. It may be changed or removed without notice in + /// any release. You should only use it directly in your code with extreme caution and knowing that + /// doing so can result in application failures when updating to a new Entity Framework Core release. + /// + public NpgsqlQueryTranslationPostprocessorFactory( + QueryTranslationPostprocessorDependencies dependencies, + RelationalQueryTranslationPostprocessorDependencies relationalDependencies) + { + Dependencies = dependencies; + RelationalDependencies = relationalDependencies; + } + + /// + /// Dependencies for this service. + /// + protected virtual QueryTranslationPostprocessorDependencies Dependencies { get; } + + /// + /// Relational provider-specific dependencies for this service. + /// + protected virtual RelationalQueryTranslationPostprocessorDependencies RelationalDependencies { get; } + + /// + /// This is an internal API that supports the Entity Framework Core infrastructure and not subject to + /// the same compatibility standards as public APIs. It may be changed or removed without notice in + /// any release. You should only use it directly in your code with extreme caution and knowing that + /// doing so can result in application failures when updating to a new Entity Framework Core release. + /// + public virtual QueryTranslationPostprocessor Create(QueryCompilationContext queryCompilationContext) + { + return new NpgsqlQueryTranslationPostprocessor( + Dependencies, + RelationalDependencies, + queryCompilationContext); + } + } +} diff --git a/src/EFCore.PG/Query/Internal/NpgsqlSqlNullabilityProcessor.cs b/src/EFCore.PG/Query/Internal/NpgsqlSqlNullabilityProcessor.cs index ef8af8bf..6805ecd8 100644 --- a/src/EFCore.PG/Query/Internal/NpgsqlSqlNullabilityProcessor.cs +++ b/src/EFCore.PG/Query/Internal/NpgsqlSqlNullabilityProcessor.cs @@ -28,38 +28,35 @@ public class NpgsqlSqlNullabilityProcessor : SqlNullabilityProcessor /// protected override SqlExpression VisitCustomSqlExpression( - SqlExpression sqlExpression, bool allowOptimizedExpansion, out bool nullable) + SqlExpression sqlExpression, + bool allowOptimizedExpansion, + out bool nullable) => sqlExpression switch { PostgresAnyExpression postgresAnyExpression - => VisitAny(postgresAnyExpression, allowOptimizedExpansion, out nullable), + => VisitAny(postgresAnyExpression, allowOptimizedExpansion, out nullable), PostgresAllExpression postgresAllExpression - => VisitAll(postgresAllExpression, allowOptimizedExpansion, out nullable), + => VisitAll(postgresAllExpression, allowOptimizedExpansion, out nullable), PostgresArrayIndexExpression arrayIndexExpression - => VisitArrayIndex(arrayIndexExpression, allowOptimizedExpansion, out nullable), + => VisitArrayIndex(arrayIndexExpression, allowOptimizedExpansion, out nullable), PostgresBinaryExpression binaryExpression - => VisitBinary(binaryExpression, allowOptimizedExpansion, out nullable), + => VisitBinary(binaryExpression, allowOptimizedExpansion, out nullable), PostgresILikeExpression ilikeExpression - => VisitILike(ilikeExpression, allowOptimizedExpansion, out nullable), + => VisitILike(ilikeExpression, allowOptimizedExpansion, out nullable), PostgresNewArrayExpression newArrayExpression - => VisitNewArray(newArrayExpression, allowOptimizedExpansion, out nullable), + => VisitNewArray(newArrayExpression, allowOptimizedExpansion, out nullable), PostgresRegexMatchExpression regexMatchExpression - => VisitRegexMatch(regexMatchExpression, allowOptimizedExpansion, out nullable), + => VisitRegexMatch(regexMatchExpression, allowOptimizedExpansion, out nullable), PostgresJsonTraversalExpression postgresJsonTraversalExpression - => VisitJsonTraversal(postgresJsonTraversalExpression, allowOptimizedExpansion, out nullable), + => VisitJsonTraversal(postgresJsonTraversalExpression, allowOptimizedExpansion, out nullable), + PostgresRowValueExpression postgresRowValueExpression + => VisitRowValueExpression(postgresRowValueExpression, allowOptimizedExpansion, out nullable), PostgresUnknownBinaryExpression postgresUnknownBinaryExpression - => VisitUnknownBinary(postgresUnknownBinaryExpression, allowOptimizedExpansion, out nullable), + => VisitUnknownBinary(postgresUnknownBinaryExpression, allowOptimizedExpansion, out nullable), _ => base.VisitCustomSqlExpression(sqlExpression, allowOptimizedExpansion, out nullable) }; - /// - /// Visits a and computes its nullability. - /// - /// A expression to visit. - /// A bool value indicating if optimized expansion which considers null value as false value is allowed. - /// A bool value indicating whether the sql expression is nullable. - /// An optimized sql expression. protected virtual SqlExpression VisitAny( PostgresAnyExpression anyExpression, bool allowOptimizedExpansion, out bool nullable) { @@ -128,13 +125,6 @@ PostgresUnknownBinaryExpression postgresUnknownBinaryExpression typeof(int))))); } - /// - /// Visits a and computes its nullability. - /// - /// A expression to visit. - /// A bool value indicating if optimized expansion which considers null value as false value is allowed. - /// A bool value indicating whether the sql expression is nullable. - /// An optimized sql expression. protected virtual SqlExpression VisitAll( PostgresAllExpression allExpression, bool allowOptimizedExpansion, out bool nullable) { @@ -157,15 +147,6 @@ PostgresUnknownBinaryExpression postgresUnknownBinaryExpression return updated; } - /// - /// Visits an and computes its nullability. - /// - /// - /// - /// A expression to visit. - /// A bool value indicating if optimized expansion which considers null value as false value is allowed. - /// A bool value indicating whether the sql expression is nullable. - /// An optimized sql expression. protected virtual SqlExpression VisitArrayIndex( PostgresArrayIndexExpression arrayIndexExpression, bool allowOptimizedExpansion, out bool nullable) { @@ -179,15 +160,6 @@ PostgresUnknownBinaryExpression postgresUnknownBinaryExpression return arrayIndexExpression.Update(array, index); } - /// - /// Visits a and computes its nullability. - /// - /// - /// - /// A expression to visit. - /// A bool value indicating if optimized expansion which considers null value as false value is allowed. - /// A bool value indicating whether the sql expression is nullable. - /// An optimized sql expression. protected virtual SqlExpression VisitBinary( PostgresBinaryExpression binaryExpression, bool allowOptimizedExpansion, out bool nullable) { @@ -213,13 +185,6 @@ PostgresUnknownBinaryExpression postgresUnknownBinaryExpression return binaryExpression.Update(left, right); } - /// - /// Visits a and computes its nullability. - /// - /// A expression to visit. - /// A bool value indicating if optimized expansion which considers null value as false value is allowed. - /// A bool value indicating whether the sql expression is nullable. - /// An optimized sql expression. protected virtual SqlExpression VisitILike( PostgresILikeExpression iLikeExpression, bool allowOptimizedExpansion, out bool nullable) { @@ -240,15 +205,6 @@ PostgresUnknownBinaryExpression postgresUnknownBinaryExpression : visited; } - /// - /// Visits a and computes its nullability. - /// - /// - /// - /// A expression to visit. - /// A bool value indicating if optimized expansion which considers null value as false value is allowed. - /// A bool value indicating whether the sql expression is nullable. - /// An optimized sql expression. protected virtual SqlExpression VisitNewArray( PostgresNewArrayExpression newArrayExpression, bool allowOptimizedExpansion, out bool nullable) { @@ -280,15 +236,6 @@ PostgresUnknownBinaryExpression postgresUnknownBinaryExpression : new PostgresNewArrayExpression(newInitializers, newArrayExpression.Type, newArrayExpression.TypeMapping); } - /// - /// Visits a and computes its nullability. - /// - /// - /// - /// A expression to visit. - /// A bool value indicating if optimized expansion which considers null value as false value is allowed. - /// A bool value indicating whether the sql expression is nullable. - /// An optimized sql expression. protected virtual SqlExpression VisitRegexMatch( PostgresRegexMatchExpression regexMatchExpression, bool allowOptimizedExpansion, out bool nullable) { @@ -302,15 +249,6 @@ PostgresUnknownBinaryExpression postgresUnknownBinaryExpression return regexMatchExpression.Update(match, pattern); } - /// - /// Visits a and computes its nullability. - /// - /// - /// - /// A expression to visit. - /// A bool value indicating if optimized expansion which considers null value as false value is allowed. - /// A bool value indicating whether the sql expression is nullable. - /// An optimized sql expression. protected virtual SqlExpression VisitJsonTraversal( PostgresJsonTraversalExpression jsonTraversalExpression, bool allowOptimizedExpansion, out bool nullable) { @@ -342,11 +280,15 @@ PostgresUnknownBinaryExpression postgresUnknownBinaryExpression // See #1851 for optimizing this for JSON POCO mapping. nullable = true; - return jsonTraversalExpression.Update( - expression, - newPath is null - ? jsonTraversalExpression.Path - : newPath.ToArray()); + return jsonTraversalExpression.Update(expression, newPath?.ToArray() ?? jsonTraversalExpression.Path); + } + + protected virtual SqlExpression VisitRowValueExpression( + PostgresRowValueExpression rowValueExpression, bool allowOptimizedExpansion, out bool nullable) + { + nullable = false; + + return rowValueExpression; } /// diff --git a/src/EFCore.PG/Query/Internal/NpgsqlSqlTranslatingExpressionVisitor.cs b/src/EFCore.PG/Query/Internal/NpgsqlSqlTranslatingExpressionVisitor.cs index 2b53f912..009c18ac 100644 --- a/src/EFCore.PG/Query/Internal/NpgsqlSqlTranslatingExpressionVisitor.cs +++ b/src/EFCore.PG/Query/Internal/NpgsqlSqlTranslatingExpressionVisitor.cs @@ -418,7 +418,17 @@ protected override Expression VisitBinary(BinaryExpression binaryExpression) _sqlExpressionFactory.ArrayIndex(sqlLeft!, _sqlExpressionFactory.GenerateOneBasedIndexExpression(sqlRight!)); } - return base.VisitBinary(binaryExpression); + var translated = base.VisitBinary(binaryExpression); + + // Translate x > 5 || (x == 5 && y > 6) -> (x, y) > (5, 6) to SQL standard + // https://www.postgresql.org/docs/current/functions-comparisons.html#ROW-WISE-COMPARISON + + if (translated is SqlBinaryExpression { OperatorType: ExpressionType.Or }) + { + + } + + return translated; } protected override Expression VisitNew(NewExpression newExpression) diff --git a/test/EFCore.PG.FunctionalTests/Query/PostgresOptimizationsQueryTest.cs b/test/EFCore.PG.FunctionalTests/Query/PostgresOptimizationsQueryTest.cs new file mode 100644 index 00000000..52d1f685 --- /dev/null +++ b/test/EFCore.PG.FunctionalTests/Query/PostgresOptimizationsQueryTest.cs @@ -0,0 +1,200 @@ +using System; +using System.Collections.Generic; +using System.Linq; +using System.Threading.Tasks; +using Microsoft.EntityFrameworkCore; +using Microsoft.EntityFrameworkCore.Query; +using Microsoft.EntityFrameworkCore.TestUtilities; +using Npgsql.EntityFrameworkCore.PostgreSQL.TestUtilities; +using Xunit; +using Xunit.Abstractions; + +namespace Npgsql.EntityFrameworkCore.PostgreSQL.Query +{ + public class PostgresOptimizationsQueryTest : QueryTestBase + { + // ReSharper disable once UnusedParameter.Local + public PostgresOptimizationsQueryTest(PostgresOptimizationsQueryFixture fixture, ITestOutputHelper testOutputHelper) + : base(fixture) + { + Fixture.TestSqlLoggerFactory.Clear(); + Fixture.TestSqlLoggerFactory.SetTestOutputHelper(testOutputHelper); + } + + [ConditionalTheory] + [MemberData(nameof(IsAsyncData))] + public async Task Row_value_comparison_two_items(bool async) + { + await AssertQuery( + async, + ss => ss.Set().Where(e => e.X > 5 || e.X == 5 && e.Y > 6), + entryCount: 1); + + AssertSql( + @"SELECT e.""Id"", e.""X"", e.""Y"", e.""Z"" +FROM ""Entities"" AS e +WHERE (e.""X"", e.""Y"") > (5, 6)"); + } + + [ConditionalTheory] + [MemberData(nameof(IsAsyncData))] + public async Task Row_value_comparison_not_rewritten_with_incompatible_operators(bool async) + { + await AssertQuery( + async, + ss => ss.Set().Where(e => e.X > 5 || e.X == 5 && e.Y < 6), + entryCount: 0); + + AssertSql( + @"SELECT e.""Id"", e.""X"", e.""Y"", e.""Z"" +FROM ""Entities"" AS e +WHERE (e.""X"" > 5) OR ((e.""X"" = 5) AND (e.""Y"" < 6))"); + } + + [ConditionalTheory] + [MemberData(nameof(IsAsyncData))] + public async Task Row_value_comparison_not_rewritten_with_incompatible_operands(bool async) + { + await AssertQuery( + async, + ss => ss.Set().Where(e => e.Z > 5 || e.X == 5 && e.Y > 6), + entryCount: 2); + + AssertSql( + @"SELECT e.""Id"", e.""X"", e.""Y"", e.""Z"" +FROM ""Entities"" AS e +WHERE (e.""Z"" > 5) OR ((e.""X"" = 5) AND (e.""Y"" > 6))"); + } + + [ConditionalTheory] + [MemberData(nameof(IsAsyncData))] + public async Task Row_value_comparison_three_items(bool async) + { + await AssertQuery( + async, + ss => ss.Set().Where(e => e.X > 5 || e.X == 5 && (e.Y > 6 || e.Y == 6 && e.Z > 7)), + entryCount: 1); + + AssertSql( + @"SELECT e.""Id"", e.""X"", e.""Y"", e.""Z"" +FROM ""Entities"" AS e +WHERE (e.""X"", e.""Y"", e.""Z"") > (5, 6, 7)"); + } + + #region Support + + private void AssertSql(params string[] expected) + => Fixture.TestSqlLoggerFactory.AssertBaseline(expected); + + public class PostgresOptimizationsQueryContext : PoolableDbContext + { + public DbSet Entities { get; set; } + + public PostgresOptimizationsQueryContext(DbContextOptions options) : base(options) {} + + public static void Seed(PostgresOptimizationsQueryContext context) + { + context.Entities.AddRange(PostgresOptimizationsData.CreateEntities()); + context.SaveChanges(); + } + } + + public class Entity + { + public int Id { get; set; } + + public int X { get; set; } + public int Y { get; set; } + public int Z { get; set; } + } + + public class PostgresOptimizationsQueryFixture : SharedStoreFixtureBase, IQueryFixtureBase + { + protected override string StoreName => "PostgresOptimizationsQueryTest"; + + // Set the PostgreSQL TimeZone parameter to something local, to ensure that operations which take TimeZone into account + // don't depend on the database's time zone, and also that operations which shouldn't take TimeZone into account indeed + // don't. + protected override ITestStoreFactory TestStoreFactory + => NpgsqlTestStoreFactory.WithConnectionStringOptions("-c TimeZone=Europe/Berlin"); + + public TestSqlLoggerFactory TestSqlLoggerFactory => (TestSqlLoggerFactory)ListLoggerFactory; + + private PostgresOptimizationsData _expectedData; + + protected override void Seed(PostgresOptimizationsQueryContext context) => PostgresOptimizationsQueryContext.Seed(context); + + public Func GetContextCreator() + => CreateContext; + + public ISetSource GetExpectedData() + => _expectedData ??= new PostgresOptimizationsData(); + + public IReadOnlyDictionary GetEntitySorters() + => new Dictionary> { { typeof(Entity), e => ((Entity)e)?.Id } } + .ToDictionary(e => e.Key, e => (object)e.Value); + + public IReadOnlyDictionary GetEntityAsserters() + => new Dictionary> + { + { + typeof(Entity), (e, a) => + { + Assert.Equal(e is null, a is null); + if (a is not null) + { + var ee = (Entity)e; + var aa = (Entity)a; + + Assert.Equal(ee.Id, aa.Id); + + Assert.Equal(ee.X, aa.X); + Assert.Equal(ee.Y, aa.Y); + Assert.Equal(ee.Z, aa.Z); + } + } + } + }.ToDictionary(e => e.Key, e => (object)e.Value); + } + + protected class PostgresOptimizationsData : ISetSource + { + public IReadOnlyList Entities { get; } + + public PostgresOptimizationsData() + => Entities = CreateEntities(); + + public IQueryable Set() + where TEntity : class + { + if (typeof(TEntity) == typeof(Entity)) + { + return (IQueryable)Entities.AsQueryable(); + } + + throw new InvalidOperationException("Invalid entity type: " + typeof(TEntity)); + } + + public static IReadOnlyList CreateEntities() + => new List + { + new() + { + Id = 1, + X = 5, + Y = 7, + Z = 9 + }, + new() + { + Id = 2, + X = 4, + Y = 10, + Z = 10 + } + }; + } + + #endregion + } +} -- GitLab