npgsql · roji · Nov 17, 2021 · Nov 17, 2021
diff --git a/src/EFCore.PG/Query/Internal/NpgsqlQuerySqlGenerator.cs b/src/EFCore.PG/Query/Internal/NpgsqlQuerySqlGenerator.cs
@@ -356,49 +356,64 @@ protected override Expression VisitSqlUnary(SqlUnaryExpression sqlUnaryExpressio
 
             switch (sqlUnaryExpression.OperatorType)
             {
-            case ExpressionType.Convert:
-            {
-                // PostgreSQL supports the standard CAST(x AS y), but also a lighter x::y which we use
-                // where there's no precedence issues
-                switch (sqlUnaryExpression.Operand)
+                case ExpressionType.Convert:
                 {
-                case SqlConstantExpression:
-                case SqlParameterExpression:
-                case SqlUnaryExpression { OperatorType: ExpressionType.Convert }:
-                case ColumnExpression:
-                case SqlFunctionExpression:
-                case ScalarSubqueryExpression:
-                    var storeType = sqlUnaryExpression.TypeMapping.StoreType switch
+                    // PostgreSQL supports the standard CAST(x AS y), but also a lighter x::y which we use
+                    // where there's no precedence issues
+                    switch (sqlUnaryExpression.Operand)
                     {
-                        "integer" => "INT",
-                        "timestamp with time zone" => "timestamptz",
-                        "timestamp without time zone" => "timestamp",
-                        var s => s
-                    };
+                        case SqlConstantExpression:
+                        case SqlParameterExpression:
+                        case SqlUnaryExpression { OperatorType: ExpressionType.Convert }:
+                        case ColumnExpression:
+                        case SqlFunctionExpression:
+                        case ScalarSubqueryExpression:
+                            var storeType = sqlUnaryExpression.TypeMapping.StoreType switch
+                            {
+                                "integer" => "INT",
+                                "timestamp with time zone" => "timestamptz",
+                                "timestamp without time zone" => "timestamp",
+                                var s => s
+                            };
+
+                            Visit(sqlUnaryExpression.Operand);
+                            Sql.Append("::");
+                            Sql.Append(storeType);
+                            return sqlUnaryExpression;
+                    }
+
+                    break;
+                }
 
+                // Bitwise complement on networking types
+                case ExpressionType.Not when
+                    sqlUnaryExpression.Operand.TypeMapping.ClrType == typeof(IPAddress)
+                    || sqlUnaryExpression.Operand.TypeMapping.ClrType == typeof((IPAddress, int))
+                    || sqlUnaryExpression.Operand.TypeMapping.ClrType == typeof(PhysicalAddress):
+                    Sql.Append("~");
                     Visit(sqlUnaryExpression.Operand);
-                    Sql.Append("::");
-                    Sql.Append(storeType);
                     return sqlUnaryExpression;
-                }
 
-                break;
-            }
+                // Not operation on full-text queries
+                case ExpressionType.Not when sqlUnaryExpression.Operand.TypeMapping.ClrType == typeof(NpgsqlTsQuery):
+                    Sql.Append("!!");
+                    Visit(sqlUnaryExpression.Operand);
+                    return sqlUnaryExpression;
 
-            // Bitwise complement on networking types
-            case ExpressionType.Not when
-                sqlUnaryExpression.Operand.TypeMapping.ClrType == typeof(IPAddress) ||
-                sqlUnaryExpression.Operand.TypeMapping.ClrType == typeof((IPAddress, int)) ||
-                sqlUnaryExpression.Operand.TypeMapping.ClrType == typeof(PhysicalAddress):
-                Sql.Append("~");
-                Visit(sqlUnaryExpression.Operand);
-                return sqlUnaryExpression;
+                // EF uses unary Equal and NotEqual to represent is-null checking.
+                // These need to be surrounded in parentheses in various cases (e.g. where TRUE = x IS NOT NULL),
+                // see
+                case ExpressionType.Equal:
+                    Sql.Append("(");
+                    Visit(sqlUnaryExpression.Operand);
+                    Sql.Append(" IS NULL)");
+                    return sqlUnaryExpression;
 
-            // Not operation on full-text queries
-            case ExpressionType.Not when sqlUnaryExpression.Operand.TypeMapping.ClrType == typeof(NpgsqlTsQuery):
-                Sql.Append("!!");
-                Visit(sqlUnaryExpression.Operand);
-                return sqlUnaryExpression;
+                case ExpressionType.NotEqual:
+                    Sql.Append("(");
+                    Visit(sqlUnaryExpression.Operand);
+                    Sql.Append(" IS NOT NULL)");
+                    return sqlUnaryExpression;
             }
 
             return base.VisitSqlUnary(sqlUnaryExpression);

diff --git a/test/EFCore.PG.FunctionalTests/Query/ArrayArrayQueryTest.cs b/test/EFCore.PG.FunctionalTests/Query/ArrayArrayQueryTest.cs
@@ -56,7 +56,7 @@ public override async Task Nullable_value_array_index_compare_to_null(bool async
             AssertSql(
                 @"SELECT s.""Id"", s.""ArrayContainerEntityId"", s.""Byte"", s.""ByteArray"", s.""Bytea"", s.""IntArray"", s.""IntList"", s.""NonNullableText"", s.""NullableIntArray"", s.""NullableIntList"", s.""NullableStringArray"", s.""NullableStringList"", s.""NullableText"", s.""StringArray"", s.""StringList"", s.""ValueConvertedArray"", s.""ValueConvertedList"", s.""ValueConvertedScalar""
 FROM ""SomeEntities"" AS s
-WHERE s.""NullableIntArray""[3] IS NULL");
+WHERE (s.""NullableIntArray""[3] IS NULL)");
         }
 
         public override async Task Non_nullable_value_array_index_compare_to_null(bool async)
@@ -76,7 +76,7 @@ public override async Task Nullable_reference_array_index_compare_to_null(bool a
             AssertSql(
                 @"SELECT s.""Id"", s.""ArrayContainerEntityId"", s.""Byte"", s.""ByteArray"", s.""Bytea"", s.""IntArray"", s.""IntList"", s.""NonNullableText"", s.""NullableIntArray"", s.""NullableIntList"", s.""NullableStringArray"", s.""NullableStringList"", s.""NullableText"", s.""StringArray"", s.""StringList"", s.""ValueConvertedArray"", s.""ValueConvertedList"", s.""ValueConvertedScalar""
 FROM ""SomeEntities"" AS s
-WHERE s.""NullableStringArray""[3] IS NULL");
+WHERE (s.""NullableStringArray""[3] IS NULL)");
         }
 
         public override async Task Non_nullable_reference_array_index_compare_to_null(bool async)
@@ -205,7 +205,7 @@ public override async Task Array_column_Contains_null_constant(bool async)
             AssertSql(
                 @"SELECT s.""Id"", s.""ArrayContainerEntityId"", s.""Byte"", s.""ByteArray"", s.""Bytea"", s.""IntArray"", s.""IntList"", s.""NonNullableText"", s.""NullableIntArray"", s.""NullableIntList"", s.""NullableStringArray"", s.""NullableStringList"", s.""NullableText"", s.""StringArray"", s.""StringList"", s.""ValueConvertedArray"", s.""ValueConvertedList"", s.""ValueConvertedScalar""
 FROM ""SomeEntities"" AS s
-WHERE array_position(s.""NullableStringArray"", NULL) IS NOT NULL");
+WHERE (array_position(s.""NullableStringArray"", NULL) IS NOT NULL)");
         }
 
         public override void Array_column_Contains_null_parameter_does_not_work()
@@ -263,7 +263,7 @@ await AssertQuery(
 
 SELECT s.""Id"", s.""ArrayContainerEntityId"", s.""Byte"", s.""ByteArray"", s.""Bytea"", s.""IntArray"", s.""IntList"", s.""NonNullableText"", s.""NullableIntArray"", s.""NullableIntList"", s.""NullableStringArray"", s.""NullableStringList"", s.""NullableText"", s.""StringArray"", s.""StringList"", s.""ValueConvertedArray"", s.""ValueConvertedList"", s.""ValueConvertedScalar""
 FROM ""SomeEntities"" AS s
-WHERE s.""NullableText"" = ANY (@__array_0) OR (s.""NullableText"" IS NULL AND array_position(@__array_0, NULL) IS NOT NULL)");
+WHERE s.""NullableText"" = ANY (@__array_0) OR ((s.""NullableText"" IS NULL) AND (array_position(@__array_0, NULL) IS NOT NULL))");
         }
 
         public override async Task Array_param_Contains_non_nullable_column(bool async)
@@ -312,7 +312,7 @@ public override void Array_param_with_null_Contains_non_nullable_not_found_negat
 
 SELECT COUNT(*)::INT
 FROM ""SomeEntities"" AS s
-WHERE NOT (s.""NonNullableText"" = ANY (@__array_0) AND (s.""NonNullableText"" = ANY (@__array_0) IS NOT NULL))");
+WHERE NOT (s.""NonNullableText"" = ANY (@__array_0) AND ((s.""NonNullableText"" = ANY (@__array_0) IS NOT NULL)))");
         }
 
         public override void Array_param_with_null_Contains_nullable_not_found()
@@ -328,7 +328,7 @@ public override void Array_param_with_null_Contains_nullable_not_found()
 
 SELECT COUNT(*)::INT
 FROM ""SomeEntities"" AS s
-WHERE s.""NullableText"" = ANY (@__array_0) OR (s.""NullableText"" IS NULL AND array_position(@__array_0, NULL) IS NOT NULL)");
+WHERE s.""NullableText"" = ANY (@__array_0) OR ((s.""NullableText"" IS NULL) AND (array_position(@__array_0, NULL) IS NOT NULL))");
         }
 
         public override void Array_param_with_null_Contains_nullable_not_found_negated()
@@ -344,7 +344,7 @@ public override void Array_param_with_null_Contains_nullable_not_found_negated()
 
 SELECT COUNT(*)::INT
 FROM ""SomeEntities"" AS s
-WHERE NOT (s.""NullableText"" = ANY (@__array_0) AND (s.""NullableText"" = ANY (@__array_0) IS NOT NULL)) AND (s.""NullableText"" IS NOT NULL OR array_position(@__array_0, NULL) IS NULL)");
+WHERE NOT (s.""NullableText"" = ANY (@__array_0) AND ((s.""NullableText"" = ANY (@__array_0) IS NOT NULL))) AND ((s.""NullableText"" IS NOT NULL) OR (array_position(@__array_0, NULL) IS NULL))");
         }
 
         public override async Task Byte_array_parameter_contains_column(bool async)

diff --git a/test/EFCore.PG.FunctionalTests/Query/ArrayListQueryTest.cs b/test/EFCore.PG.FunctionalTests/Query/ArrayListQueryTest.cs
@@ -59,7 +59,7 @@ public override async Task Nullable_value_array_index_compare_to_null(bool async
             AssertSql(
                 @"SELECT s.""Id"", s.""ArrayContainerEntityId"", s.""Byte"", s.""ByteArray"", s.""Bytea"", s.""IntArray"", s.""IntList"", s.""NonNullableText"", s.""NullableIntArray"", s.""NullableIntList"", s.""NullableStringArray"", s.""NullableStringList"", s.""NullableText"", s.""StringArray"", s.""StringList"", s.""ValueConvertedArray"", s.""ValueConvertedList"", s.""ValueConvertedScalar""
 FROM ""SomeEntities"" AS s
-WHERE s.""NullableIntList""[3] IS NULL");
+WHERE (s.""NullableIntList""[3] IS NULL)");
         }
 
         public override async Task Non_nullable_value_array_index_compare_to_null(bool async)
@@ -79,7 +79,7 @@ public override async Task Nullable_reference_array_index_compare_to_null(bool a
             AssertSql(
                 @"SELECT s.""Id"", s.""ArrayContainerEntityId"", s.""Byte"", s.""ByteArray"", s.""Bytea"", s.""IntArray"", s.""IntList"", s.""NonNullableText"", s.""NullableIntArray"", s.""NullableIntList"", s.""NullableStringArray"", s.""NullableStringList"", s.""NullableText"", s.""StringArray"", s.""StringList"", s.""ValueConvertedArray"", s.""ValueConvertedList"", s.""ValueConvertedScalar""
 FROM ""SomeEntities"" AS s
-WHERE s.""NullableStringList""[3] IS NULL");
+WHERE (s.""NullableStringList""[3] IS NULL)");
         }
 
         public override async Task Non_nullable_reference_array_index_compare_to_null(bool async)
@@ -193,7 +193,7 @@ public override async Task Array_column_Contains_null_constant(bool async)
             AssertSql(
                 @"SELECT s.""Id"", s.""ArrayContainerEntityId"", s.""Byte"", s.""ByteArray"", s.""Bytea"", s.""IntArray"", s.""IntList"", s.""NonNullableText"", s.""NullableIntArray"", s.""NullableIntList"", s.""NullableStringArray"", s.""NullableStringList"", s.""NullableText"", s.""StringArray"", s.""StringList"", s.""ValueConvertedArray"", s.""ValueConvertedList"", s.""ValueConvertedScalar""
 FROM ""SomeEntities"" AS s
-WHERE array_position(s.""NullableStringList"", NULL) IS NOT NULL");
+WHERE (array_position(s.""NullableStringList"", NULL) IS NOT NULL)");
         }
 
         public override void Array_column_Contains_null_parameter_does_not_work()
@@ -251,7 +251,8 @@ await AssertQuery(
 
 SELECT s.""Id"", s.""ArrayContainerEntityId"", s.""Byte"", s.""ByteArray"", s.""Bytea"", s.""IntArray"", s.""IntList"", s.""NonNullableText"", s.""NullableIntArray"", s.""NullableIntList"", s.""NullableStringArray"", s.""NullableStringList"", s.""NullableText"", s.""StringArray"", s.""StringList"", s.""ValueConvertedArray"", s.""ValueConvertedList"", s.""ValueConvertedScalar""
 FROM ""SomeEntities"" AS s
-WHERE s.""NullableText"" = ANY (@__array_0) OR (s.""NullableText"" IS NULL AND array_position(@__array_0, NULL) IS NOT NULL)");
+WHERE s.""NullableText"" = ANY (@__array_0) OR ((s.""NullableText"" IS NULL) AND (array_position(@__array_0, NULL) IS NOT NULL))");
+
         }
 
         public override async Task Array_param_Contains_non_nullable_column(bool async)
@@ -310,7 +311,7 @@ public override void Array_param_with_null_Contains_non_nullable_not_found_negat
 
 SELECT COUNT(*)::INT
 FROM ""SomeEntities"" AS s
-WHERE NOT (s.""NonNullableText"" = ANY (@__array_0) AND (s.""NonNullableText"" = ANY (@__array_0) IS NOT NULL))");
+WHERE NOT (s.""NonNullableText"" = ANY (@__array_0) AND ((s.""NonNullableText"" = ANY (@__array_0) IS NOT NULL)))");
         }
 
         public override void Array_param_with_null_Contains_nullable_not_found()
@@ -331,7 +332,7 @@ public override void Array_param_with_null_Contains_nullable_not_found()
 
 SELECT COUNT(*)::INT
 FROM ""SomeEntities"" AS s
-WHERE s.""NullableText"" = ANY (@__array_0) OR (s.""NullableText"" IS NULL AND array_position(@__array_0, NULL) IS NOT NULL)");
+WHERE s.""NullableText"" = ANY (@__array_0) OR ((s.""NullableText"" IS NULL) AND (array_position(@__array_0, NULL) IS NOT NULL))");
         }
 
         public override void Array_param_with_null_Contains_nullable_not_found_negated()
@@ -352,7 +353,7 @@ public override void Array_param_with_null_Contains_nullable_not_found_negated()
 
 SELECT COUNT(*)::INT
 FROM ""SomeEntities"" AS s
-WHERE NOT (s.""NullableText"" = ANY (@__array_0) AND (s.""NullableText"" = ANY (@__array_0) IS NOT NULL)) AND (s.""NullableText"" IS NOT NULL OR array_position(@__array_0, NULL) IS NULL)");
+WHERE NOT (s.""NullableText"" = ANY (@__array_0) AND ((s.""NullableText"" = ANY (@__array_0) IS NOT NULL))) AND ((s.""NullableText"" IS NOT NULL) OR (array_position(@__array_0, NULL) IS NULL))");
         }
 
         public override async Task Byte_array_parameter_contains_column(bool async)

diff --git a/test/EFCore.PG.FunctionalTests/Query/CitextQueryTest.cs b/test/EFCore.PG.FunctionalTests/Query/CitextQueryTest.cs
@@ -20,7 +20,7 @@ public CitextQueryTest(CitextQueryFixture fixture, ITestOutputHelper testOutputH
         {
             Fixture = fixture;
             Fixture.TestSqlLoggerFactory.Clear();
-            //Fixture.TestSqlLoggerFactory.SetTestOutputHelper(testOutputHelper);
+            // Fixture.TestSqlLoggerFactory.SetTestOutputHelper(testOutputHelper);
         }
 
         [Fact]
@@ -33,7 +33,7 @@ public void StartsWith_literal()
             AssertSql(
                 @"SELECT s.""Id"", s.""CaseInsensitiveText""
 FROM ""SomeEntities"" AS s
-WHERE s.""CaseInsensitiveText"" IS NOT NULL AND (s.""CaseInsensitiveText"" LIKE 'some%')
+WHERE (s.""CaseInsensitiveText"" IS NOT NULL) AND (s.""CaseInsensitiveText"" LIKE 'some%')
 LIMIT 2");
         }
 
@@ -50,7 +50,7 @@ public void StartsWith_param_pattern()
 
 SELECT s.""Id"", s.""CaseInsensitiveText""
 FROM ""SomeEntities"" AS s
-WHERE (@__param_0 = '') OR (s.""CaseInsensitiveText"" IS NOT NULL AND ((s.""CaseInsensitiveText"" LIKE @__param_0 || '%' ESCAPE '') AND (left(s.""CaseInsensitiveText"", length(@__param_0))::citext = @__param_0::citext)))
+WHERE (@__param_0 = '') OR ((s.""CaseInsensitiveText"" IS NOT NULL) AND ((s.""CaseInsensitiveText"" LIKE @__param_0 || '%' ESCAPE '') AND (left(s.""CaseInsensitiveText"", length(@__param_0))::citext = @__param_0::citext)))
 LIMIT 2");
         }
 
@@ -67,7 +67,7 @@ public void StartsWith_param_instance()
 
 SELECT s.""Id"", s.""CaseInsensitiveText""
 FROM ""SomeEntities"" AS s
-WHERE (s.""CaseInsensitiveText"" = '') OR (s.""CaseInsensitiveText"" IS NOT NULL AND ((@__param_0 LIKE s.""CaseInsensitiveText"" || '%' ESCAPE '') AND (left(@__param_0, length(s.""CaseInsensitiveText""))::citext = s.""CaseInsensitiveText""::citext)))
+WHERE (s.""CaseInsensitiveText"" = '') OR ((s.""CaseInsensitiveText"" IS NOT NULL) AND ((@__param_0 LIKE s.""CaseInsensitiveText"" || '%' ESCAPE '') AND (left(@__param_0, length(s.""CaseInsensitiveText""))::citext = s.""CaseInsensitiveText""::citext)))
 LIMIT 2");
         }
 
@@ -81,7 +81,7 @@ public void EndsWith_literal()
             AssertSql(
                 @"SELECT s.""Id"", s.""CaseInsensitiveText""
 FROM ""SomeEntities"" AS s
-WHERE s.""CaseInsensitiveText"" IS NOT NULL AND (s.""CaseInsensitiveText"" LIKE '%sometext')
+WHERE (s.""CaseInsensitiveText"" IS NOT NULL) AND (s.""CaseInsensitiveText"" LIKE '%sometext')
 LIMIT 2");
         }
 
@@ -98,7 +98,7 @@ public void EndsWith_param_pattern()
 
 SELECT s.""Id"", s.""CaseInsensitiveText""
 FROM ""SomeEntities"" AS s
-WHERE (@__param_0 = '') OR (s.""CaseInsensitiveText"" IS NOT NULL AND (right(s.""CaseInsensitiveText"", length(@__param_0))::citext = @__param_0::citext))
+WHERE (@__param_0 = '') OR ((s.""CaseInsensitiveText"" IS NOT NULL) AND (right(s.""CaseInsensitiveText"", length(@__param_0))::citext = @__param_0::citext))
 LIMIT 2");
         }
 
@@ -115,7 +115,7 @@ public void EndsWith_param_instance()
 
 SELECT s.""Id"", s.""CaseInsensitiveText""
 FROM ""SomeEntities"" AS s
-WHERE (s.""CaseInsensitiveText"" = '') OR (s.""CaseInsensitiveText"" IS NOT NULL AND (right(@__param_0, length(s.""CaseInsensitiveText""))::citext = s.""CaseInsensitiveText""::citext))
+WHERE (s.""CaseInsensitiveText"" = '') OR ((s.""CaseInsensitiveText"" IS NOT NULL) AND (right(@__param_0, length(s.""CaseInsensitiveText""))::citext = s.""CaseInsensitiveText""::citext))
 LIMIT 2");
         }
 

diff --git a/test/EFCore.PG.FunctionalTests/Query/JsonDomQueryTest.cs b/test/EFCore.PG.FunctionalTests/Query/JsonDomQueryTest.cs
@@ -310,7 +310,7 @@ public void Like()
             AssertSql(
                 @"SELECT j.""Id"", j.""CustomerDocument"", j.""CustomerElement""
 FROM ""JsonbEntities"" AS j
-WHERE j.""CustomerElement""->>'Name' IS NOT NULL AND (j.""CustomerElement""->>'Name' LIKE 'J%')
+WHERE (j.""CustomerElement""->>'Name' IS NOT NULL) AND (j.""CustomerElement""->>'Name' LIKE 'J%')
 LIMIT 2");
         }
 

diff --git a/test/EFCore.PG.FunctionalTests/Query/JsonPocoQueryTest.cs b/test/EFCore.PG.FunctionalTests/Query/JsonPocoQueryTest.cs
@@ -214,7 +214,7 @@ public void Compare_to_null()
             AssertSql(
                 @"SELECT j.""Id"", j.""Customer"", j.""ToplevelArray""
 FROM ""JsonbEntities"" AS j
-WHERE j.""Customer""#>>'{Statistics,Nested,SomeNullableInt}' IS NULL
+WHERE (j.""Customer""#>>'{Statistics,Nested,SomeNullableInt}' IS NULL)
 LIMIT 2");
         }
 
@@ -357,7 +357,7 @@ public void Like()
             AssertSql(
                 @"SELECT j.""Id"", j.""Customer"", j.""ToplevelArray""
 FROM ""JsonbEntities"" AS j
-WHERE j.""Customer""->>'Name' IS NOT NULL AND (j.""Customer""->>'Name' LIKE 'J%')
+WHERE (j.""Customer""->>'Name' IS NOT NULL) AND (j.""Customer""->>'Name' LIKE 'J%')
 LIMIT 2");
         }
 

diff --git a/test/EFCore.PG.FunctionalTests/Query/NorthwindFunctionsQueryNpgsqlTest.cs b/test/EFCore.PG.FunctionalTests/Query/NorthwindFunctionsQueryNpgsqlTest.cs
@@ -28,7 +28,7 @@ public override async Task IsNullOrWhiteSpace_in_predicate(bool async)
             AssertSql(
                 @"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 c.""Region"" IS NULL OR (btrim(c.""Region"", E' \t\n\r') = '')");
+WHERE (c.""Region"" IS NULL) OR (btrim(c.""Region"", E' \t\n\r') = '')");
         }
 
         public override Task Where_math_log_new_base(bool async)