From 4a1c87b1e1b9ddf89abef1c148b722883eda0646 Mon Sep 17 00:00:00 2001 From: Shay Rojansky Date: Tue, 16 May 2023 23:15:43 +0200 Subject: [PATCH] Correct parentheses logic for SQLite JsonScalarExpression Fixes #30886 --- .../Query/QuerySqlGenerator.cs | 2 +- .../Internal/SqlServerQuerySqlGenerator.cs | 3 ++ .../Query/OperatorsQueryTestBase.cs | 36 ++++++++++++++++++- ...SharedPrimitiveCollectionsQueryTestBase.cs | 20 +++++------ .../Query/OperatorsQuerySqlServerTest.cs | 12 +++++++ .../Query/OperatorsQuerySqliteTest.cs | 13 +++++++ 6 files changed, 72 insertions(+), 14 deletions(-) diff --git a/src/EFCore.Relational/Query/QuerySqlGenerator.cs b/src/EFCore.Relational/Query/QuerySqlGenerator.cs index efac1cd435b..4be62a43408 100644 --- a/src/EFCore.Relational/Query/QuerySqlGenerator.cs +++ b/src/EFCore.Relational/Query/QuerySqlGenerator.cs @@ -1533,7 +1533,7 @@ protected virtual bool RequiresParentheses(SqlExpression outerExpression, SqlExp return true; } - case CollateExpression or LikeExpression or AtTimeZoneExpression: + case CollateExpression or LikeExpression or AtTimeZoneExpression or JsonScalarExpression: return !TryGetOperatorInfo(outerExpression, out outerPrecedence, out _) || !TryGetOperatorInfo(innerExpression, out innerPrecedence, out _) || outerPrecedence >= innerPrecedence; diff --git a/src/EFCore.SqlServer/Query/Internal/SqlServerQuerySqlGenerator.cs b/src/EFCore.SqlServer/Query/Internal/SqlServerQuerySqlGenerator.cs index bdd5ac3eeee..285cf69f723 100644 --- a/src/EFCore.SqlServer/Query/Internal/SqlServerQuerySqlGenerator.cs +++ b/src/EFCore.SqlServer/Query/Internal/SqlServerQuerySqlGenerator.cs @@ -591,6 +591,9 @@ protected override bool TryGetOperatorInfo(SqlExpression expression, out int pre LikeExpression => (350, false), AtTimeZoneExpression => (1200, false), + // On SQL Server, JsonScalarExpression renders as a function (JSON_VALUE()), so there's never a need for parentheses. + JsonScalarExpression => (9999, false), + _ => default, }; diff --git a/test/EFCore.Relational.Specification.Tests/Query/OperatorsQueryTestBase.cs b/test/EFCore.Relational.Specification.Tests/Query/OperatorsQueryTestBase.cs index a94a4c24a4d..d2f4d3c7654 100644 --- a/test/EFCore.Relational.Specification.Tests/Query/OperatorsQueryTestBase.cs +++ b/test/EFCore.Relational.Specification.Tests/Query/OperatorsQueryTestBase.cs @@ -169,7 +169,7 @@ from e4 in context.Set() from e5 in context.Set() orderby e3.Id, e4.Id, e5.Id select ((~(-(-((e5.Value + e3.Value) + 2))) % (-(e4.Value + e4.Value) - e3.Value)))).ToList(); - + Assert.Equal(expected.Count, actual.Count); for (var i = 0; i < expected.Count; i++) { @@ -267,4 +267,38 @@ public virtual async Task Negate_on_like_expression(bool async) Assert.Equal(expected[i], actual[i]); } } + +#nullable enable + [ConditionalTheory] + [MemberData(nameof(IsAsyncData))] + public virtual async Task Concat_and_json_scalar(bool async) + { + var contextFactory = await InitializeAsync( + onModelCreating: mb => mb + .Entity() + .OwnsOne(o => o.Owned) + .ToJson(), + seed: context => + { + context.Set().AddRange( + new Owner { Owned = new() { SomeProperty = "Bar" } }, + new Owner { Owned = new() { SomeProperty = "Baz" } }); + context.SaveChanges(); + }); + await using var context = contextFactory.CreateContext(); + + var result = await context.Set().SingleAsync(o => "Foo" + o.Owned.SomeProperty == "FooBar"); + Assert.Equal("Bar", result.Owned.SomeProperty); + } + + class Owner + { + public int Id { get; set; } + public Owned Owned { get; set; } = null!; + } + + class Owned + { + public string SomeProperty { get; set; } = ""; + } } diff --git a/test/EFCore.Specification.Tests/Query/NonSharedPrimitiveCollectionsQueryTestBase.cs b/test/EFCore.Specification.Tests/Query/NonSharedPrimitiveCollectionsQueryTestBase.cs index 4780f98ee64..f8bd4d0c201 100644 --- a/test/EFCore.Specification.Tests/Query/NonSharedPrimitiveCollectionsQueryTestBase.cs +++ b/test/EFCore.Specification.Tests/Query/NonSharedPrimitiveCollectionsQueryTestBase.cs @@ -131,12 +131,10 @@ public virtual async Task Column_with_custom_converter() public virtual async Task Parameter_with_inferred_value_converter() { var contextFactory = await InitializeAsync( - onModelCreating: mb => mb.Entity( - b => - { - b.Property("PropertyWithValueConverter") - .HasConversion(w => w.Value, i => new IntWrapper(i)); - }), + onModelCreating: mb => mb + .Entity() + .Property("PropertyWithValueConverter") + .HasConversion(w => w.Value, i => new IntWrapper(i)), seed: context => { var entry1 = context.Add(new TestEntity { Id = 1 }); @@ -158,12 +156,10 @@ public virtual async Task Parameter_with_inferred_value_converter() public virtual async Task Constant_with_inferred_value_converter() { var contextFactory = await InitializeAsync( - onModelCreating: mb => mb.Entity( - b => - { - b.Property("PropertyWithValueConverter") - .HasConversion(w => w.Value, i => new IntWrapper(i)); - }), + onModelCreating: mb => mb + .Entity() + .Property("PropertyWithValueConverter") + .HasConversion(w => w.Value, i => new IntWrapper(i)), seed: context => { var entry1 = context.Add(new TestEntity { Id = 1 }); diff --git a/test/EFCore.SqlServer.FunctionalTests/Query/OperatorsQuerySqlServerTest.cs b/test/EFCore.SqlServer.FunctionalTests/Query/OperatorsQuerySqlServerTest.cs index 5dfa3d018fb..0b30a5f2f55 100644 --- a/test/EFCore.SqlServer.FunctionalTests/Query/OperatorsQuerySqlServerTest.cs +++ b/test/EFCore.SqlServer.FunctionalTests/Query/OperatorsQuerySqlServerTest.cs @@ -128,6 +128,18 @@ WHERE [o].[Value] IS NOT NULL AND NOT ([o].[Value] LIKE N'A%') """); } + public override async Task Concat_and_json_scalar(bool async) + { + await base.Concat_and_json_scalar(async); + + AssertSql( +""" +SELECT TOP(2) [o].[Id], [o].[Owned] +FROM [Owner] AS [o] +WHERE N'Foo' + JSON_VALUE([o].[Owned], '$.SomeProperty') = N'FooBar' +"""); + } + [ConditionalTheory] [MemberData(nameof(IsAsyncData))] public virtual async Task Where_AtTimeZone_datetimeoffset_constant(bool async) diff --git a/test/EFCore.Sqlite.FunctionalTests/Query/OperatorsQuerySqliteTest.cs b/test/EFCore.Sqlite.FunctionalTests/Query/OperatorsQuerySqliteTest.cs index 7d38580dad9..91a74647a6d 100644 --- a/test/EFCore.Sqlite.FunctionalTests/Query/OperatorsQuerySqliteTest.cs +++ b/test/EFCore.Sqlite.FunctionalTests/Query/OperatorsQuerySqliteTest.cs @@ -117,6 +117,19 @@ public override async Task Negate_on_like_expression(bool async) SELECT "o"."Id" FROM "OperatorEntityString" AS "o" WHERE "o"."Value" IS NOT NULL AND NOT ("o"."Value" LIKE 'A%') +"""); + } + + public override async Task Concat_and_json_scalar(bool async) + { + await base.Concat_and_json_scalar(async); + + AssertSql( +""" +SELECT "o"."Id", "o"."Owned" +FROM "Owner" AS "o" +WHERE 'Foo' || ("o"."Owned" ->> 'SomeProperty') = 'FooBar' +LIMIT 2 """); } }