Use precedence-aware parentheses for regex operands

src/EFCore.PG/Query/Internal/NpgsqlQuerySqlGenerator.cs

@@ -963,9 +963,20 @@ protected virtual Expression VisitArraySlice(PgArraySliceExpression expression)
     protected virtual Expression VisitRegexMatch(PgRegexMatchExpression expression, bool negated = false)
     {
         var options = expression.Options;
+        var requiresParentheses = RequiresParentheses(expression, expression.Match);
+
+        if (requiresParentheses)
+        {
+            Sql.Append("(");
+        }
 
         Visit(expression.Match);
 
+        if (requiresParentheses)
+        {
+            Sql.Append(")");
+        }
+
         if (options.HasFlag(RegexOptions.IgnoreCase))
         {
             Sql.Append(negated ? " !~* " : " ~* ");
@@ -976,10 +987,24 @@ protected virtual Expression VisitRegexMatch(PgRegexMatchExpression expression,
             Sql.Append(negated ? " !~ " : " ~ ");
         }
 
+        // Regex matching and concatenation have the same precedence, including after the option prefix below.
+        requiresParentheses = RequiresParentheses(expression, expression.Pattern);
+
         // PG regexps are single-line by default
         if (options == RegexOptions.Singleline)
         {
+            if (requiresParentheses)
+            {
+                Sql.Append("(");
+            }
+
             Visit(expression.Pattern);
+
+            if (requiresParentheses)
+            {
+                Sql.Append(")");
+            }
+
             return expression;
         }
 
@@ -1012,7 +1037,19 @@ protected virtual Expression VisitRegexMatch(PgRegexMatchExpression expression,
         if (constantPattern is null)
         {
             Sql.Append("' || ");
+
+            if (requiresParentheses)
+            {
+                Sql.Append("(");
+            }
+
             Visit(expression.Pattern);
+
+            if (requiresParentheses)
+            {
+                Sql.Append(")");
+            }
+
             Sql.Append(")");
         }
         else
@@ -1493,6 +1530,13 @@ protected override bool RequiresParentheses(SqlExpression outerExpression, SqlEx
                 return true;
             }
 
+            case PgJsonTraversalExpression when outerExpression is not SqlUnaryExpression { OperatorType: ExpressionType.Convert }:
+            {
+                return !TryGetOperatorInfo(outerExpression, out var outerPrecedence, out _)
+                    || !TryGetOperatorInfo(innerExpression, out var innerPrecedence, out _)
+                    || outerPrecedence >= innerPrecedence;
+            }
+
             // PG requires function calls to be wrapped in parentheses before indexing on the returned array:
             // (string_to_array(c."ContactName", ' '))[1]
             case SqlFunctionExpression when outerExpression is PgArrayIndexExpression:
@@ -1524,6 +1568,9 @@ protected override bool TryGetOperatorInfo(SqlExpression expression, out int pre
                 ExpressionType.Divide => (1200, false),
                 ExpressionType.Modulo => (1200, false),
 
+                // String, array and tsvector concatenation
+                ExpressionType.Add when GetOperator(sqlBinaryExpression) == " || " => (1000, true),
+
                 // Addition, subtraction (binary)
                 ExpressionType.Add => (1100, true),
                 ExpressionType.Subtract => (1100, false),
@@ -1565,15 +1612,15 @@ protected override bool TryGetOperatorInfo(SqlExpression expression, out int pre
 
             // There's an "any other operator" category in the PG operator precedence table, we assign that a numeric value of 1000.
             // TODO: Some operators here may be associative
-            PgBinaryExpression => (1000, false),
+            PgBinaryExpression or PgRegexMatchExpression => (1000, false),
 
             CollateExpression => (1000, false),
             AtTimeZoneExpression => (1100, false),
             InExpression => (900, false),
             PgJsonTraversalExpression => (1000, false),
             PgArrayIndexExpression => (1500, false),
             PgAllExpression or PgAnyExpression => (800, false),
-            LikeExpression or PgILikeExpression or PgRegexMatchExpression => (900, false),
+            LikeExpression or PgILikeExpression => (900, false),
 
             _ => default,
         };

test/EFCore.PG.FunctionalTests/Query/JsonDomQueryTest.cs

@@ -1,5 +1,6 @@
 using System.ComponentModel.DataAnnotations.Schema;
 using System.Text.Json;
+using System.Text.RegularExpressions;
 
 namespace Microsoft.EntityFrameworkCore.Query;
 
@@ -511,7 +512,7 @@ public void JsonExists()
             """
 SELECT count(*)::int
 FROM "JsonbEntities" AS j
-WHERE j."CustomerElement" -> 'Statistics' ? 'Visits'
+WHERE (j."CustomerElement" -> 'Statistics') ? 'Visits'
 """);
     }
 
@@ -528,7 +529,7 @@ public void JsonExistAny()
             """
 SELECT count(*)::int
 FROM "JsonbEntities" AS j
-WHERE j."CustomerElement" -> 'Statistics' ?| ARRAY['foo','Visits']::text[]
+WHERE (j."CustomerElement" -> 'Statistics') ?| ARRAY['foo','Visits']::text[]
 """);
     }
 
@@ -545,7 +546,7 @@ public void JsonExistAll()
             """
 SELECT count(*)::int
 FROM "JsonbEntities" AS j
-WHERE j."CustomerElement" -> 'Statistics' ?& ARRAY['foo','Visits']::text[]
+WHERE (j."CustomerElement" -> 'Statistics') ?& ARRAY['foo','Visits']::text[]
 """);
     }
 
@@ -585,6 +586,110 @@ WHERE json_typeof(j."CustomerElement" #> '{Statistics,Visits}') = 'number'
 
     #endregion Functions
 
+    [Theory]
+    [InlineData(false)]
+    [InlineData(true)]
+    public void Regex_with_json_pattern(bool jsonb)
+    {
+        using var ctx = CreateContext();
+
+        var query = jsonb
+            ? ctx.JsonbEntities.Select(e => new { e.Id, e.CustomerElement })
+            : ctx.JsonEntities.Select(e => new { e.Id, e.CustomerElement });
+
+        var ids = query.Where(e => Regex.IsMatch("Joe", e.CustomerElement.GetProperty("Name").GetString()!))
+            .Select(e => e.Id)
+            .ToList();
+
+        Assert.Equal([1], ids);
+        AssertSql(
+            $"""
+SELECT j."Id"
+FROM "{(jsonb ? "JsonbEntities" : "JsonEntities")}" AS j
+WHERE 'Joe' ~ ('(?p)' || (j."CustomerElement" ->> 'Name'))
+""");
+    }
+
+    [Theory]
+    [InlineData(RegexOptions.Singleline, false, "~ (j.\"CustomerElement\" ->> 'Name')")]
+    [InlineData(RegexOptions.IgnoreCase, false, "~* ('(?p)' || (j.\"CustomerElement\" ->> 'Name'))")]
+    [InlineData(RegexOptions.Singleline | RegexOptions.IgnoreCase, false, "~* (j.\"CustomerElement\" ->> 'Name')")]
+    [InlineData(RegexOptions.None, true, "!~ ('(?p)' || (j.\"CustomerElement\" ->> 'Name'))")]
+    [InlineData(RegexOptions.Singleline, true, "!~ (j.\"CustomerElement\" ->> 'Name')")]
+    [InlineData(RegexOptions.IgnoreCase, true, "!~* ('(?p)' || (j.\"CustomerElement\" ->> 'Name'))")]
+    [InlineData(RegexOptions.Singleline | RegexOptions.IgnoreCase, true, "!~* (j.\"CustomerElement\" ->> 'Name')")]
+    public void Regex_with_json_pattern_options(RegexOptions options, bool negated, string expectedOperatorAndPattern)
+    {
+        using var ctx = CreateContext();
+
+        var query = ctx.JsonbEntities.Where(e => e.Id == 1);
+        var matches = options switch
+        {
+            RegexOptions.None
+                => query.Select(e => Regex.IsMatch("Joe", e.CustomerElement.GetProperty("Name").GetString()!)),
+            RegexOptions.Singleline
+                => query.Select(e => Regex.IsMatch("Joe", e.CustomerElement.GetProperty("Name").GetString()!, RegexOptions.Singleline)),
+            RegexOptions.IgnoreCase
+                => query.Select(e => Regex.IsMatch("Joe", e.CustomerElement.GetProperty("Name").GetString()!, RegexOptions.IgnoreCase)),
+            RegexOptions.Singleline | RegexOptions.IgnoreCase
+                => query.Select(e => Regex.IsMatch(
+                    "Joe", e.CustomerElement.GetProperty("Name").GetString()!, RegexOptions.Singleline | RegexOptions.IgnoreCase)),
+            _ => throw new ArgumentOutOfRangeException(nameof(options))
+        };
+        var result = (negated ? matches.Select(m => !m) : matches).Single();
+
+        Assert.Equal(!negated, result);
+        AssertSql(
+            $"""
+SELECT 'Joe' {expectedOperatorAndPattern}
+FROM "JsonbEntities" AS j
+WHERE j."Id" = 1
+LIMIT 2
+""");
+    }
+
+    [Fact]
+    public void Regex_with_json_input()
+    {
+        using var ctx = CreateContext();
+
+        var ids = ctx.JsonbEntities
+            .Where(e => Regex.IsMatch(e.CustomerElement.GetProperty("Name").GetString()!, "^J"))
+            .Select(e => e.Id)
+            .ToList();
+
+        Assert.Equal([1], ids);
+        AssertSql(
+            """
+SELECT j."Id"
+FROM "JsonbEntities" AS j
+WHERE (j."CustomerElement" ->> 'Name') ~ '(?p)^J'
+""");
+    }
+
+    [Fact]
+    public void Regex_with_nested_json_operands()
+    {
+        using var ctx = CreateContext();
+
+        var ids = ctx.JsonbEntities
+            .Where(e => Regex.IsMatch(
+                e.CustomerElement.GetProperty("VariousTypes").GetProperty("String").GetString()!,
+                e.CustomerElement.GetProperty("VariousTypes").GetProperty("String").GetString()!))
+            .OrderBy(e => e.Id)
+            .Select(e => e.Id)
+            .ToList();
+
+        Assert.Equal([1, 2], ids);
+        AssertSql(
+            """
+SELECT j."Id"
+FROM "JsonbEntities" AS j
+WHERE (j."CustomerElement" #>> '{VariousTypes,String}') ~ ('(?p)' || (j."CustomerElement" #>> '{VariousTypes,String}'))
+ORDER BY j."Id" NULLS FIRST
+""");
+    }
+
     #region Support
 
     protected JsonDomQueryContext CreateContext()

test/EFCore.PG.FunctionalTests/Query/JsonPocoQueryTest.cs

@@ -620,7 +620,7 @@ public void JsonExists()
             """
 SELECT count(*)::int
 FROM "JsonbEntities" AS j
-WHERE j."Customer" -> 'Statistics' ? 'Visits'
+WHERE (j."Customer" -> 'Statistics') ? 'Visits'
 """);
     }
 
@@ -637,7 +637,7 @@ public void JsonExistAny()
             """
 SELECT count(*)::int
 FROM "JsonbEntities" AS j
-WHERE j."Customer" -> 'Statistics' ?| ARRAY['foo','Visits']::text[]
+WHERE (j."Customer" -> 'Statistics') ?| ARRAY['foo','Visits']::text[]
 """);
     }
 
@@ -654,7 +654,7 @@ public void JsonExistAll()
             """
 SELECT count(*)::int
 FROM "JsonbEntities" AS j
-WHERE j."Customer" -> 'Statistics' ?& ARRAY['foo','Visits']::text[]
+WHERE (j."Customer" -> 'Statistics') ?& ARRAY['foo','Visits']::text[]
 """);
     }
 

test/EFCore.PG.FunctionalTests/Query/JsonQueryNpgsqlTest.cs

@@ -1,3 +1,4 @@
+using System.Text.RegularExpressions;
 using Microsoft.EntityFrameworkCore.TestModels.JsonQuery;
 using Xunit.Sdk;
 
@@ -3687,6 +3688,22 @@ FROM ROWS FROM (jsonb_to_recordset(p."OrderIds") AS ("Value" integer)) WITH ORDI
 
     #endregion
 
+    [Theory]
+    [MemberData(nameof(IsAsyncData))]
+    public async Task Regex_with_json_operands(bool async)
+    {
+        await AssertQuery(
+            async,
+            ss => ss.Set<JsonEntityBasic>()
+                .Select(e => Regex.IsMatch(e.OwnedReferenceRoot.Name, e.OwnedReferenceRoot.Name)));
+
+        AssertSql(
+            """
+SELECT (j."OwnedReferenceRoot" ->> 'Name') ~ ('(?p)' || (j."OwnedReferenceRoot" ->> 'Name'))
+FROM "JsonEntitiesBasic" AS j
+""");
+    }
+
     [ConditionalFact]
     public virtual void Check_all_tests_overridden()
         => TestHelpers.AssertAllMethodsOverridden(GetType());

test/EFCore.PG.FunctionalTests/Query/NorthwindFunctionsQueryNpgsqlTest.cs

@@ -105,6 +105,38 @@ await AssertQuery(
 """);
     }
 
+    [Theory]
+    [MemberData(nameof(IsAsyncData))]
+    public async Task Regex_IsMatch_with_concatenated_input(bool async)
+    {
+        await AssertQuery(
+            async,
+            cs => cs.Set<Customer>().Where(c => Regex.IsMatch(c.CustomerID + "x", "^A")));
+
+        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."CustomerID" || 'x') ~ '(?p)^A'
+""");
+    }
+
+    [Theory]
+    [MemberData(nameof(IsAsyncData))]
+    public async Task Regex_IsMatch_with_concatenated_pattern_Singleline(bool async)
+    {
+        await AssertQuery(
+            async,
+            cs => cs.Set<Customer>().Where(c => Regex.IsMatch(c.CustomerID, c.CustomerID + "$", RegexOptions.Singleline)));
+
+        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."CustomerID" ~ (c."CustomerID" || '$')
+""");
+    }
+
     [Theory]
     [MemberData(nameof(IsAsyncData))]
     public async Task Regex_IsMatch_negated(bool async)