Correct parentheses logic for SQLite JsonScalarExpression

Fixes dotnet#30886
roji · May 16, 2023 · 4a1c87b · 4a1c87b
1 parent ee74a60
commit 4a1c87b
Show file tree

Hide file tree

Showing 6 changed files with 72 additions and 14 deletions.
diff --git 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
@@ -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
@@ -169,7 +169,7 @@ from e4 in context.Set<OperatorEntityLong>()
                       from e5 in context.Set<OperatorEntityLong>()
                       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<DbContext>(
+            onModelCreating: mb => mb
+                .Entity<Owner>()
+                .OwnsOne(o => o.Owned)
+                .ToJson(),
+            seed: context =>
+            {
+                context.Set<Owner>().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<Owner>().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
@@ -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<TestContext>(
-            onModelCreating: mb => mb.Entity<TestEntity>(
-                b =>
-                {
-                    b.Property<IntWrapper>("PropertyWithValueConverter")
-                        .HasConversion(w => w.Value, i => new IntWrapper(i));
-                }),
+            onModelCreating: mb => mb
+                .Entity<TestEntity>()
+                .Property<IntWrapper>("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<TestContext>(
-            onModelCreating: mb => mb.Entity<TestEntity>(
-                b =>
-                {
-                    b.Property<IntWrapper>("PropertyWithValueConverter")
-                        .HasConversion(w => w.Value, i => new IntWrapper(i));
-                }),
+            onModelCreating: mb => mb
+                .Entity<TestEntity>()
+                .Property<IntWrapper>("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
@@ -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
@@ -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
 """);
     }
 }