Remove COALESCE from array ANY expression

Because it prevents index usage. Will reimplement after we can sniff the parameter for null values. Fixes #1152
npgsql · Dec 13, 2019 · 03b63f2 · 03b63f2
1 parent 6786e9c
commit 03b63f2
Show file tree

Hide file tree

Showing 4 changed files with 17 additions and 13 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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