From 03b63f20d08405842f2d30c38415fece6f394831 Mon Sep 17 00:00:00 2001 From: Shay Rojansky Date: Wed, 11 Dec 2019 12:56:13 +0100 Subject: [PATCH] Remove COALESCE from array ANY expression Because it prevents index usage. Will reimplement after we can sniff the parameter for null values. Fixes #1152 --- .../Internal/NpgsqlArrayMethodTranslator.cs | 9 ++++----- test/EFCore.PG.FunctionalTests/Query/ArrayQueryTest.cs | 6 +++--- .../Query/NullSemanticsQueryNpgsqlTest.cs | 7 ++++++- .../Query/SimpleQueryNpgsqlTest.cs | 8 ++++---- 4 files changed, 17 insertions(+), 13 deletions(-) diff --git a/src/EFCore.PG/Query/ExpressionTranslators/Internal/NpgsqlArrayMethodTranslator.cs b/src/EFCore.PG/Query/ExpressionTranslators/Internal/NpgsqlArrayMethodTranslator.cs index aa65c7769..edab868c2 100644 --- a/src/EFCore.PG/Query/ExpressionTranslators/Internal/NpgsqlArrayMethodTranslator.cs +++ b/src/EFCore.PG/Query/ExpressionTranslators/Internal/NpgsqlArrayMethodTranslator.cs @@ -101,14 +101,13 @@ public SqlExpression Translate(SqlExpression instance, MethodInfo method, IReadO Nullable.GetUnderlyingType(operandElementType) == null) { var item = arguments[1]; + // TODO: no null semantics is implemented here (see https://github.com/npgsql/efcore.pg/issues/1142) // We require a null semantics check in case the item is null and the array contains a null. // Advanced parameter sniffing would help here: https://github.com/aspnet/EntityFrameworkCore/issues/17598 + // We need to coalesce to false since 'x' = ANY ({'y', NULL}) returns null, not false + // (and so will be null when negated too) return _sqlExpressionFactory.OrElse( - // We need to coalesce to false since 'x' = ANY ({'y', NULL}) returns null, not false - // (and so will be null when negated too) - _sqlExpressionFactory.Coalesce( - _sqlExpressionFactory.ArrayAnyAll(item, operand, ArrayComparisonType.Any, "="), - _sqlExpressionFactory.Constant(false)), + _sqlExpressionFactory.ArrayAnyAll(item, operand, ArrayComparisonType.Any, "="), _sqlExpressionFactory.AndAlso( _sqlExpressionFactory.IsNull(item), _sqlExpressionFactory.IsNotNull( diff --git a/test/EFCore.PG.FunctionalTests/Query/ArrayQueryTest.cs b/test/EFCore.PG.FunctionalTests/Query/ArrayQueryTest.cs index b817a4113..4dcfa5374 100644 --- a/test/EFCore.PG.FunctionalTests/Query/ArrayQueryTest.cs +++ b/test/EFCore.PG.FunctionalTests/Query/ArrayQueryTest.cs @@ -140,7 +140,7 @@ public void Contains_with_literal() AssertSql( @"SELECT s.""Id"", s.""SomeArray"", s.""SomeBytea"", s.""SomeList"", s.""SomeMatrix"", s.""SomeText"" FROM ""SomeEntities"" AS s -WHERE COALESCE(3 = ANY (s.""SomeArray""), FALSE) +WHERE 3 = ANY (s.""SomeArray"") LIMIT 2"); } @@ -158,7 +158,7 @@ public void Contains_with_parameter() SELECT s.""Id"", s.""SomeArray"", s.""SomeBytea"", s.""SomeList"", s.""SomeMatrix"", s.""SomeText"" FROM ""SomeEntities"" AS s -WHERE COALESCE(@__p_0 = ANY (s.""SomeArray""), FALSE) +WHERE @__p_0 = ANY (s.""SomeArray"") LIMIT 2"); } @@ -172,7 +172,7 @@ public void Contains_with_column() AssertSql( @"SELECT s.""Id"", s.""SomeArray"", s.""SomeBytea"", s.""SomeList"", s.""SomeMatrix"", s.""SomeText"" FROM ""SomeEntities"" AS s -WHERE COALESCE(s.""Id"" + 2 = ANY (s.""SomeArray""), FALSE) +WHERE s.""Id"" + 2 = ANY (s.""SomeArray"") LIMIT 2"); } diff --git a/test/EFCore.PG.FunctionalTests/Query/NullSemanticsQueryNpgsqlTest.cs b/test/EFCore.PG.FunctionalTests/Query/NullSemanticsQueryNpgsqlTest.cs index c56a5d1f3..b39723aab 100644 --- a/test/EFCore.PG.FunctionalTests/Query/NullSemanticsQueryNpgsqlTest.cs +++ b/test/EFCore.PG.FunctionalTests/Query/NullSemanticsQueryNpgsqlTest.cs @@ -1,7 +1,9 @@ -using Microsoft.EntityFrameworkCore; +using System.Threading.Tasks; +using Microsoft.EntityFrameworkCore; using Microsoft.EntityFrameworkCore.TestModels.NullSemanticsModel; using Microsoft.EntityFrameworkCore.Query; using Npgsql.EntityFrameworkCore.PostgreSQL.Infrastructure; +using Xunit; namespace Npgsql.EntityFrameworkCore.PostgreSQL.Query { @@ -12,6 +14,9 @@ public NullSemanticsQueryNpgsqlTest(NullSemanticsQueryNpgsqlFixture fixture) : base(fixture) => Fixture.TestSqlLoggerFactory.Clear(); + [ConditionalFact(Skip = "Null semantics for array ANY not yet implemented, #1142")] + public override void Contains_with_local_array_closure_false_with_null() {} + protected override void ClearLog() => Fixture.TestSqlLoggerFactory.Clear(); diff --git a/test/EFCore.PG.FunctionalTests/Query/SimpleQueryNpgsqlTest.cs b/test/EFCore.PG.FunctionalTests/Query/SimpleQueryNpgsqlTest.cs index be20818e6..979778224 100644 --- a/test/EFCore.PG.FunctionalTests/Query/SimpleQueryNpgsqlTest.cs +++ b/test/EFCore.PG.FunctionalTests/Query/SimpleQueryNpgsqlTest.cs @@ -64,13 +64,13 @@ public override async Task Contains_with_local_uint_array_closure(bool isAsync) SELECT e.""EmployeeID"", e.""City"", e.""Country"", e.""FirstName"", e.""ReportsTo"", e.""Title"" FROM ""Employees"" AS e -WHERE COALESCE(e.""EmployeeID"" = ANY (@__ids_0), FALSE)", +WHERE e.""EmployeeID"" = ANY (@__ids_0)", // @"@__ids_0='System.Int64[]' (DbType = Object) SELECT e.""EmployeeID"", e.""City"", e.""Country"", e.""FirstName"", e.""ReportsTo"", e.""Title"" FROM ""Employees"" AS e -WHERE COALESCE(e.""EmployeeID"" = ANY (@__ids_0), FALSE)"); +WHERE e.""EmployeeID"" = ANY (@__ids_0)"); } public override async Task Contains_with_local_nullable_uint_array_closure(bool isAsync) @@ -251,7 +251,7 @@ await AssertQuery( SELECT c.""CustomerID"", c.""Address"", c.""City"", c.""CompanyName"", c.""ContactName"", c.""ContactTitle"", c.""Country"", c.""Fax"", c.""Phone"", c.""PostalCode"", c.""Region"" FROM ""Customers"" AS c -WHERE COALESCE(c.""Region"" = ANY (@__regions_0), FALSE) OR ((c.""Region"" IS NULL) AND (array_position(@__regions_0, NULL) IS NOT NULL))"); +WHERE c.""Region"" = ANY (@__regions_0) OR ((c.""Region"" IS NULL) AND (array_position(@__regions_0, NULL) IS NOT NULL))"); } [ConditionalTheory] @@ -274,7 +274,7 @@ await AssertQuery( SELECT c.""CustomerID"", c.""Address"", c.""City"", c.""CompanyName"", c.""ContactName"", c.""ContactTitle"", c.""Country"", c.""Fax"", c.""Phone"", c.""PostalCode"", c.""Region"" FROM ""Customers"" AS c -WHERE COALESCE(c.""Region"" = ANY (@__regions_0), FALSE) OR ((c.""Region"" IS NULL) AND (array_position(@__regions_0, NULL) IS NOT NULL))"); +WHERE c.""Region"" = ANY (@__regions_0) OR ((c.""Region"" IS NULL) AND (array_position(@__regions_0, NULL) IS NOT NULL))"); } #endregion Array contains