diff --git a/src/EFCore.PG/Query/Internal/NpgsqlQuerySqlGenerator.cs b/src/EFCore.PG/Query/Internal/NpgsqlQuerySqlGenerator.cs index 5c1ed24f12..78573c00be 100644 --- a/src/EFCore.PG/Query/Internal/NpgsqlQuerySqlGenerator.cs +++ b/src/EFCore.PG/Query/Internal/NpgsqlQuerySqlGenerator.cs @@ -933,9 +933,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 ? " !~* " : " ~* "); @@ -946,10 +957,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; } @@ -982,7 +1007,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 @@ -1452,6 +1489,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; + } + case PgUnknownBinaryExpression: return true; @@ -1480,6 +1524,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), @@ -1521,7 +1568,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), @@ -1529,7 +1576,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 a46fd3c968..568c37323c 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; using Npgsql.EntityFrameworkCore.PostgreSQL.TestUtilities; namespace Npgsql.EntityFrameworkCore.PostgreSQL.Query; @@ -513,7 +514,7 @@ public void JsonExists() """ SELECT count(*)::int FROM "JsonbEntities" AS j -WHERE j."CustomerElement" -> 'Statistics' ? 'Visits' +WHERE (j."CustomerElement" -> 'Statistics') ? 'Visits' """); } @@ -530,7 +531,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[] """); } @@ -547,7 +548,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[] """); } @@ -587,6 +588,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 bb108fc6ee..ceed54eea7 100644 --- a/test/EFCore.PG.FunctionalTests/Query/JsonPocoQueryTest.cs +++ b/test/EFCore.PG.FunctionalTests/Query/JsonPocoQueryTest.cs @@ -622,7 +622,7 @@ public void JsonExists() """ SELECT count(*)::int FROM "JsonbEntities" AS j -WHERE j."Customer" -> 'Statistics' ? 'Visits' +WHERE (j."Customer" -> 'Statistics') ? 'Visits' """); } @@ -639,7 +639,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[] """); } @@ -656,7 +656,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 d1e3305b6d..d9130ddcc8 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 Npgsql.EntityFrameworkCore.PostgreSQL.TestUtilities; @@ -3386,6 +3387,22 @@ public override async Task Json_projection_second_element_projected_before_owner """); } + [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 43c78f8c20..dad1259eed 100644 --- a/test/EFCore.PG.FunctionalTests/Query/NorthwindFunctionsQueryNpgsqlTest.cs +++ b/test/EFCore.PG.FunctionalTests/Query/NorthwindFunctionsQueryNpgsqlTest.cs @@ -148,6 +148,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)