From 68c89fd7ccf57314ebe227cba8f903ce2dc1e5fd Mon Sep 17 00:00:00 2001 From: Shay Rojansky Date: Sun, 4 Oct 2026 11:23:15 +0200 Subject: [PATCH] Fix regex operand parentheses based on SQL precedence Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- .../Query/Internal/NpgsqlQuerySqlGenerator.cs | 51 +++++++- .../Query/JsonDomQueryTest.cs | 111 +++++++++++++++++- .../Query/JsonPocoQueryTest.cs | 6 +- .../Query/JsonQueryNpgsqlTest.cs | 17 +++ .../NorthwindFunctionsQueryNpgsqlTest.cs | 32 +++++ 5 files changed, 209 insertions(+), 8 deletions(-) diff --git a/src/EFCore.PG/Query/Internal/NpgsqlQuerySqlGenerator.cs b/src/EFCore.PG/Query/Internal/NpgsqlQuerySqlGenerator.cs index 9a82c8eedc..cec62234a0 100644 --- a/src/EFCore.PG/Query/Internal/NpgsqlQuerySqlGenerator.cs +++ b/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,7 +1612,7 @@ 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), @@ -1573,7 +1620,7 @@ protected override bool TryGetOperatorInfo(SqlExpression expression, out int pre PgJsonTraversalExpression => (1000, false), PgArrayIndexExpression => (1500, false), PgAllExpression or PgAnyExpression => (800, false), - LikeExpression or PgILikeExpression or PgRegexMatchExpression => (900, false), + LikeExpression or PgILikeExpression => (900, false), _ => default, }; diff --git a/test/EFCore.PG.FunctionalTests/Query/JsonDomQueryTest.cs b/test/EFCore.PG.FunctionalTests/Query/JsonDomQueryTest.cs index 525a61df1f..85dc105b7c 100644 --- a/test/EFCore.PG.FunctionalTests/Query/JsonDomQueryTest.cs +++ b/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() diff --git a/test/EFCore.PG.FunctionalTests/Query/JsonPocoQueryTest.cs b/test/EFCore.PG.FunctionalTests/Query/JsonPocoQueryTest.cs index 15994daab5..b57118049b 100644 --- a/test/EFCore.PG.FunctionalTests/Query/JsonPocoQueryTest.cs +++ b/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[] """); } diff --git a/test/EFCore.PG.FunctionalTests/Query/JsonQueryNpgsqlTest.cs b/test/EFCore.PG.FunctionalTests/Query/JsonQueryNpgsqlTest.cs index 1efd67ac9e..a49371153a 100644 --- a/test/EFCore.PG.FunctionalTests/Query/JsonQueryNpgsqlTest.cs +++ b/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() + .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()); diff --git a/test/EFCore.PG.FunctionalTests/Query/NorthwindFunctionsQueryNpgsqlTest.cs b/test/EFCore.PG.FunctionalTests/Query/NorthwindFunctionsQueryNpgsqlTest.cs index fe360fda2d..0ac5c79759 100644 --- a/test/EFCore.PG.FunctionalTests/Query/NorthwindFunctionsQueryNpgsqlTest.cs +++ b/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().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().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)