diff --git a/src/EFCore.PG/Query/Internal/NpgsqlQuerySqlGenerator.cs b/src/EFCore.PG/Query/Internal/NpgsqlQuerySqlGenerator.cs index 87aab70b81..469ed80857 100644 --- a/src/EFCore.PG/Query/Internal/NpgsqlQuerySqlGenerator.cs +++ b/src/EFCore.PG/Query/Internal/NpgsqlQuerySqlGenerator.cs @@ -908,9 +908,20 @@ protected virtual Expression VisitArraySlice(PgArraySliceExpression expression) protected virtual Expression VisitRegexMatch(PgRegexMatchExpression expression) { 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(" ~* "); @@ -921,10 +932,24 @@ protected virtual Expression VisitRegexMatch(PgRegexMatchExpression expression) Sql.Append(" ~ "); } + // 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; } @@ -957,7 +982,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 @@ -1421,6 +1458,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; @@ -1449,6 +1493,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), @@ -1490,7 +1537,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), @@ -1498,7 +1545,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 a0f3e99808..19654c1257 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; @@ -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, "'Joe' ~ (j.\"CustomerElement\" ->> 'Name')")] + [InlineData(RegexOptions.IgnoreCase, false, "'Joe' ~* ('(?p)' || (j.\"CustomerElement\" ->> 'Name'))")] + [InlineData(RegexOptions.Singleline | RegexOptions.IgnoreCase, false, "'Joe' ~* (j.\"CustomerElement\" ->> 'Name')")] + [InlineData(RegexOptions.None, true, "NOT ('Joe' ~ ('(?p)' || (j.\"CustomerElement\" ->> 'Name')))")] + [InlineData(RegexOptions.Singleline, true, "NOT ('Joe' ~ (j.\"CustomerElement\" ->> 'Name'))")] + [InlineData(RegexOptions.IgnoreCase, true, "NOT ('Joe' ~* ('(?p)' || (j.\"CustomerElement\" ->> 'Name')))")] + [InlineData(RegexOptions.Singleline | RegexOptions.IgnoreCase, true, "NOT ('Joe' ~* (j.\"CustomerElement\" ->> 'Name'))")] + public void Regex_with_json_pattern_options(RegexOptions options, bool negated, string expectedExpression) + { + 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 {expectedExpression} +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 f804c60401..b5d1594dcc 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 fe87655b73..8928c1fce8 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; @@ -3006,6 +3007,22 @@ ORDER BY m."Id" NULLS FIRST """); } + [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 0c9198a671..f5c81cfc46 100644 --- a/test/EFCore.PG.FunctionalTests/Query/NorthwindFunctionsQueryNpgsqlTest.cs +++ b/test/EFCore.PG.FunctionalTests/Query/NorthwindFunctionsQueryNpgsqlTest.cs @@ -155,6 +155,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_IsMatchOptionsNone(bool async)