Transform JSON_VALUE() to OPENJSON/WITH when needed (#33440)

Closes #33435

EFCore.sln.DotSettings

@@ -216,6 +216,7 @@ The .NET Foundation licenses this file to you under the MIT license.
 	<s:String x:Key="/Default/CodeStyle/Naming/CSharpNaming/PredefinedNamingRules/=PrivateStaticReadonly/@EntryIndexedValue">&lt;Policy Inspect="True" Prefix="" Suffix="" Style="AaBb" /&gt;</s:String>
 	<s:String x:Key="/Default/CodeStyle/Naming/CSharpNaming/UserRules/=042e7460_002D3ec6_002D4f1c_002D87c3_002Dfc91240d4c90/@EntryIndexedValue">&lt;Policy&gt;&lt;Descriptor Staticness="Static, Instance" AccessRightKinds="Public" Description="Test Methods"&gt;&lt;ElementKinds&gt;&lt;Kind Name="TEST_MEMBER" /&gt;&lt;/ElementKinds&gt;&lt;/Descriptor&gt;&lt;Policy Inspect="True" Prefix="" Suffix="" Style="Aa_bb" /&gt;&lt;/Policy&gt;</s:String>
 	<s:String x:Key="/Default/CodeStyle/Naming/CSharpNaming/Abbreviations/=EF/@EntryIndexedValue">EF</s:String>
+	<s:String x:Key="/Default/CodeStyle/Naming/CSharpNaming/UserRules/=15b5b1f1_002D457c_002D4ca6_002Db278_002D5615aedc07d3/@EntryIndexedValue">&lt;Policy&gt;&lt;Descriptor Staticness="Static" AccessRightKinds="Private" Description="Static readonly fields (private)"&gt;&lt;ElementKinds&gt;&lt;Kind Name="READONLY_FIELD" /&gt;&lt;/ElementKinds&gt;&lt;/Descriptor&gt;&lt;Policy Inspect="True" Prefix="" Suffix="" Style="AaBb" /&gt;&lt;/Policy&gt;</s:String>
 	<s:String x:Key="/Default/CodeStyle/Naming/JavaScriptNaming/UserRules/=JS_005FBLOCK_005FSCOPE_005FCONSTANT/@EntryIndexedValue">&lt;Policy Inspect="True" Prefix="" Suffix="" Style="aaBb" /&gt;</s:String>
 	<s:String x:Key="/Default/CodeStyle/Naming/JavaScriptNaming/UserRules/=JS_005FBLOCK_005FSCOPE_005FFUNCTION/@EntryIndexedValue">&lt;Policy Inspect="True" Prefix="" Suffix="" Style="aaBb" /&gt;</s:String>
 	<s:String x:Key="/Default/CodeStyle/Naming/JavaScriptNaming/UserRules/=JS_005FBLOCK_005FSCOPE_005FVARIABLE/@EntryIndexedValue">&lt;Policy Inspect="True" Prefix="" Suffix="" Style="aaBb" /&gt;</s:String>
@@ -280,6 +281,7 @@ The .NET Foundation licenses this file to you under the MIT license.&#xD;
 	<s:Boolean x:Key="/Default/Environment/SettingsMigration/IsMigratorApplied/=JetBrains_002EReSharper_002EPsi_002ECSharp_002ECodeStyle_002ESettingsUpgrade_002ECSharpPlaceAttributeOnSameLineMigration/@EntryIndexedValue">True</s:Boolean>
 	<s:Boolean x:Key="/Default/Environment/SettingsMigration/IsMigratorApplied/=JetBrains_002EReSharper_002EPsi_002ECSharp_002ECodeStyle_002ESettingsUpgrade_002EMigrateBlankLinesAroundFieldToBlankLinesAroundProperty/@EntryIndexedValue">True</s:Boolean>
 	<s:Boolean x:Key="/Default/Environment/SettingsMigration/IsMigratorApplied/=JetBrains_002EReSharper_002EPsi_002ECSharp_002ECodeStyle_002ESettingsUpgrade_002EMigrateThisQualifierSettings/@EntryIndexedValue">True</s:Boolean>
+	<s:Boolean x:Key="/Default/Environment/SettingsMigration/IsMigratorApplied/=JetBrains_002EReSharper_002EPsi_002ECSharp_002ECodeStyle_002ESettingsUpgrade_002EPredefinedNamingRulesToUserRulesUpgrade/@EntryIndexedValue">True</s:Boolean>
 	<s:Boolean x:Key="/Default/Environment/SettingsMigration/IsMigratorApplied/=JetBrains_002EReSharper_002EPsi_002EJavaScript_002ECodeStyle_002ESettingsUpgrade_002EJsCodeFormatterSettingsUpgrader/@EntryIndexedValue">True</s:Boolean>
 	<s:Boolean x:Key="/Default/Environment/SettingsMigration/IsMigratorApplied/=JetBrains_002EReSharper_002EPsi_002EJavaScript_002ECodeStyle_002ESettingsUpgrade_002EJsParsFormattingSettingsUpgrader/@EntryIndexedValue">True</s:Boolean>
 	<s:Boolean x:Key="/Default/Environment/SettingsMigration/IsMigratorApplied/=JetBrains_002EReSharper_002EPsi_002EJavaScript_002ECodeStyle_002ESettingsUpgrade_002EJsWrapperSettingsUpgrader/@EntryIndexedValue">True</s:Boolean>

src/EFCore.SqlServer/Query/Internal/SqlServerJsonPostprocessor.cs

@@ -10,23 +10,28 @@
 namespace Microsoft.EntityFrameworkCore.SqlServer.Query.Internal;
 
 /// <summary>
-///     Converts <see cref="SqlServerOpenJsonExpression" /> expressions with WITH (the default) to OPENJSON without WITH under the following
-///     conditions:
-///     * When an ordering still exists on the [key] column, i.e. when the ordering of the original JSON array needs to be preserved
-///       (e.g. limit/offset).
-///     * When the column type in the WITH clause is a SQL Server "CLR type" - these are incompatible with WITH (e.g. hierarchy id).
+///     Performs various post-processing rewriting to account for SQL Server JSON quirks.
+///     1. Converts <see cref="SqlServerOpenJsonExpression" /> expressions with WITH (the default) to OPENJSON without WITH under the
+///        following conditions:
+///         * When an ordering still exists on the [key] column, i.e. when the ordering of the original JSON array needs to be preserved
+///           (e.g. limit/offset).
+///         * When the column type in the WITH clause is a SQL Server "CLR type" - these are incompatible with WITH (e.g. hierarchy id).
+///     2. Rewrite JsonScalarExpression (JSON_VALUE()) to OPENJSON for when JSON_VALUE() isn't compatible with the type (e.g. binary data
+///        which needs to be base64-decoded).
 /// </summary>
 /// <remarks>
 ///     This is an internal API that supports the Entity Framework Core infrastructure and not subject to
 ///     the same compatibility standards as public APIs. It may be changed or removed without notice in
 ///     any release. You should only use it directly in your code with extreme caution and knowing that
 ///     doing so can result in application failures when updating to a new Entity Framework Core release.
 /// </remarks>
-public class SqlServerJsonPostprocessor : ExpressionVisitor
+public sealed class SqlServerJsonPostprocessor(
+    IRelationalTypeMappingSource typeMappingSource,
+    ISqlExpressionFactory sqlExpressionFactory,
+    SqlAliasManager sqlAliasManager)
+    : ExpressionVisitor
 {
-    private readonly IRelationalTypeMappingSource _typeMappingSource;
-    private readonly ISqlExpressionFactory _sqlExpressionFactory;
-
+    private readonly List<OuterApplyExpression> _openjsonOuterAppliesToAdd = new();
     private readonly Dictionary<(string, string), ColumnInfo> _columnsToRewrite = new();
 
     private RelationalTypeMapping? _nvarcharMaxTypeMapping;
@@ -37,20 +42,7 @@ public class SqlServerJsonPostprocessor : ExpressionVisitor
     ///     any release. You should only use it directly in your code with extreme caution and knowing that
     ///     doing so can result in application failures when updating to a new Entity Framework Core release.
     /// </summary>
-    public SqlServerJsonPostprocessor(
-        IRelationalTypeMappingSource typeMappingSource,
-        ISqlExpressionFactory sqlExpressionFactory)
-    {
-        (_typeMappingSource, _sqlExpressionFactory) = (typeMappingSource, sqlExpressionFactory);
-    }
-
-    /// <summary>
-    ///     This is an internal API that supports the Entity Framework Core infrastructure and not subject to
-    ///     the same compatibility standards as public APIs. It may be changed or removed without notice in
-    ///     any release. You should only use it directly in your code with extreme caution and knowing that
-    ///     doing so can result in application failures when updating to a new Entity Framework Core release.
-    /// </summary>
-    public virtual Expression Process(Expression expression)
+    public Expression Process(Expression expression)
     {
         _columnsToRewrite.Clear();
 
@@ -129,39 +121,60 @@ public virtual Expression Process(Expression expression)
                     }
                 }
 
+                var result = selectExpression;
+
                 // In the common case, we do not have to rewrite any OPENJSON tables.
                 if (columnsToRewrite is null)
                 {
                     Check.DebugAssert(newTables is null, "newTables must be null if columnsToRewrite is null");
-                    return base.Visit(selectExpression);
-                }
 
-                var newSelectExpression = newTables is not null
-                    ? selectExpression.Update(
-                        selectExpression.Projection,
-                        newTables,
-                        selectExpression.Predicate,
-                        selectExpression.GroupBy,
-                        selectExpression.Having,
-                        selectExpression.Orderings,
-                        selectExpression.Limit,
-                        selectExpression.Offset)
-                    : selectExpression;
-
-                // when we mark columns for rewrite we don't yet have the updated SelectExpression, so we store the info in temporary dictionary
-                // and now that we have created new SelectExpression we add it to the proper dictionary that we will use for rewrite
-                foreach (var columnToRewrite in columnsToRewrite)
-                {
-                    _columnsToRewrite.Add(columnToRewrite.Key, columnToRewrite.Value);
+                    result = (SelectExpression)base.Visit(result);
                 }
+                else
+                {
+                    if (newTables is not null)
+                    {
+                        result = selectExpression.Update(
+                            selectExpression.Projection,
+                            newTables,
+                            selectExpression.Predicate,
+                            selectExpression.GroupBy,
+                            selectExpression.Having,
+                            selectExpression.Orderings,
+                            selectExpression.Limit,
+                            selectExpression.Offset);
+                    }
 
-                // Record the OPENJSON expression and its projected column(s), along with the store type we just removed from the WITH
-                // clause. Then visit the select expression, adding a cast around the matching ColumnExpressions.
-                var result = base.Visit(newSelectExpression);
+                    // when we mark columns for rewrite we don't yet have the updated SelectExpression, so we store the info in temporary dictionary
+                    // and now that we have created new SelectExpression we add it to the proper dictionary that we will use for rewrite
+                    foreach (var columnToRewrite in columnsToRewrite)
+                    {
+                        _columnsToRewrite.Add(columnToRewrite.Key, columnToRewrite.Value);
+                    }
+
+                    // Record the OPENJSON expression and its projected column(s), along with the store type we just removed from the WITH
+                    // clause. Then visit the select expression, adding a cast around the matching ColumnExpressions.
+                    result = (SelectExpression)base.Visit(result);
 
-                foreach (var columnsToRewriteKey in columnsToRewrite.Keys)
+                    foreach (var columnsToRewriteKey in columnsToRewrite.Keys)
+                    {
+                        _columnsToRewrite.Remove(columnsToRewriteKey);
+                    }
+                }
+
+                if (_openjsonOuterAppliesToAdd.Count > 0)
                 {
-                    _columnsToRewrite.Remove(columnsToRewriteKey);
+                    result = result.Update(
+                        result.Projection,
+                        [.. result.Tables, .. _openjsonOuterAppliesToAdd],
+                        result.Predicate,
+                        result.GroupBy,
+                        result.Having,
+                        result.Orderings,
+                        result.Limit,
+                        result.Offset);
+
+                    _openjsonOuterAppliesToAdd.Clear();
                 }
 
                 return result;
@@ -198,6 +211,31 @@ public virtual Expression Process(Expression expression)
                     jsonScalarExpression.IsNullable);
             }
 
+            // Some SQL Server types cannot be reliably parsed with JSON_VALUE(): binary/varbinary are encoded in base64 in the JSON,
+            // but JSON_VALUE() returns a string and there's no SQL Server function to parse base64. However, OPENJSON/WITH does do base64
+            // decoding.
+            // So here we identify problematic JsonScalarExpressions (which translate to JSON_VALUE), and transform them to OUTER APPLY:
+            // JSON_VALUE([b].[Json], '$.Foo.Bar') -> CROSS APPLY OPENJSON([b].[Json]) WITH ([Bar] int '$.Foo.Bar') AS [b].
+            case JsonScalarExpression { TypeMapping.StoreTypeNameBase: "varbinary" or "binary" } jsonScalar:
+            {
+                var name = jsonScalar.Path.LastOrDefault(ps => ps.PropertyName is not null).PropertyName
+                           ?? (jsonScalar.Json as ColumnExpression)?.Name
+                           ?? "Json";
+
+                var tableAlias = sqlAliasManager.GenerateTableAlias(name);
+                var join =
+                    new OuterApplyExpression(
+                        new SqlServerOpenJsonExpression(
+                            tableAlias, jsonScalar.Json, path: null,
+                            columnInfos: [new(name, jsonScalar.TypeMapping, jsonScalar.Path)]));
+
+                // We record the new OUTER APPLY in _openWithOuterAppliesToAdd (it gets added after visiting the SelectExpression above),
+                // and return a ColumnExpression referencing that new OUTER APPLY.
+                _openjsonOuterAppliesToAdd.Add(join);
+                return new ColumnExpression(name, tableAlias, jsonScalar.Type, jsonScalar.TypeMapping,
+                    jsonScalar.IsNullable);
+            }
+
             default:
                 return base.Visit(expression);
         }
@@ -240,9 +278,9 @@ SqlExpression RewriteOpenJsonColumn(ColumnExpression columnExpression, ColumnInf
                 // nvarchar(max); add a CAST to convert.
                 if (columnInfo.TypeMapping.StoreType != "nvarchar(max)")
                 {
-                    _nvarcharMaxTypeMapping ??= _typeMappingSource.FindMapping("nvarchar(max)");
+                    _nvarcharMaxTypeMapping ??= typeMappingSource.FindMapping("nvarchar(max)");
 
-                    rewrittenColumn = _sqlExpressionFactory.Convert(
+                    rewrittenColumn = sqlExpressionFactory.Convert(
                         rewrittenColumn,
                         columnExpression.Type,
                         columnInfo.TypeMapping);

src/EFCore.SqlServer/Query/Internal/SqlServerQueryTranslationPostprocessor.cs

@@ -33,7 +33,7 @@ public class SqlServerQueryTranslationPostprocessor : RelationalQueryTranslation
         : base(dependencies, relationalDependencies, queryCompilationContext)
     {
         _jsonPostprocessor = new SqlServerJsonPostprocessor(
-            relationalDependencies.TypeMappingSource, relationalDependencies.SqlExpressionFactory);
+            relationalDependencies.TypeMappingSource, relationalDependencies.SqlExpressionFactory, queryCompilationContext.SqlAliasManager);
     }
 
     /// <summary>

test/EFCore.Relational.Specification.Tests/Query/JsonQueryTestBase.cs

@@ -2262,6 +2262,14 @@ public virtual Task Json_predicate_on_byte(bool async)
             async,
             ss => ss.Set<JsonEntityAllTypes>().Where(x => x.Reference.TestByte != 3));
 
+    [ConditionalTheory]
+    [MemberData(nameof(IsAsyncData))]
+    public virtual Task Json_predicate_on_byte_array(bool async)
+        => AssertQuery(
+            async,
+            ss => ss.Set<JsonEntityAllTypes>().Where(x => x.Reference.TestByteArray != new byte[] { 1, 2, 3 }),
+            ss => ss.Set<JsonEntityAllTypes>().Where(x => !x.Reference.TestByteArray.SequenceEqual(new byte[] { 1, 2, 3 })));
+
     [ConditionalTheory]
     [MemberData(nameof(IsAsyncData))]
     public virtual Task Json_predicate_on_character(bool async)

test/EFCore.Relational.Specification.Tests/TestModels/JsonQuery/JsonOwnedAllTypes.cs

@@ -34,6 +34,7 @@ public class JsonOwnedAllTypes
     public float TestSingle { get; set; }
     public bool TestBoolean { get; set; }
     public byte TestByte { get; set; }
+    public byte[] TestByteArray { get; set; }
     public Guid TestGuid { get; set; }
     public ushort TestUnsignedInt16 { get; set; }
     public uint TestUnsignedInt32 { get; set; }

test/EFCore.Relational.Specification.Tests/TestModels/JsonQuery/JsonQueryData.cs

@@ -706,6 +706,7 @@ public static IReadOnlyList<JsonEntityAllTypes> CreateJsonEntitiesAllTypes()
             TestSingle = -1.234F,
             TestBoolean = true,
             TestByte = 255,
+            TestByteArray = [1, 2, 3],
             TestGuid = new Guid("12345678-1234-4321-7777-987654321000"),
             TestUnsignedInt16 = 1234,
             TestUnsignedInt32 = 1234565789U,
@@ -825,6 +826,7 @@ public static IReadOnlyList<JsonEntityAllTypes> CreateJsonEntitiesAllTypes()
             TestSingle = -1.24F,
             TestBoolean = true,
             TestByte = 25,
+            TestByteArray = [1, 2, 4],
             TestGuid = new Guid("12345678-1243-4321-7777-987654321000"),
             TestUnsignedInt16 = 134,
             TestUnsignedInt32 = 123565789U,
@@ -939,6 +941,7 @@ public static IReadOnlyList<JsonEntityAllTypes> CreateJsonEntitiesAllTypes()
             TestSingle = -1.4F,
             TestBoolean = false,
             TestByte = 25,
+            TestByteArray = [],
             TestGuid = new Guid("00000000-0000-0000-0000-000000000000"),
             TestUnsignedInt16 = 12,
             TestUnsignedInt32 = 12345U,
@@ -1053,6 +1056,7 @@ public static IReadOnlyList<JsonEntityAllTypes> CreateJsonEntitiesAllTypes()
             TestSingle = -1.4F,
             TestBoolean = false,
             TestByte = 25,
+            TestByteArray = null,
             TestGuid = new Guid("00000000-0000-0000-0000-000000100000"),
             TestUnsignedInt16 = 1,
             TestUnsignedInt32 = 1245U,

test/EFCore.SqlServer.FunctionalTests/Query/JsonQuerySqlServerTest.cs

@@ -2297,6 +2297,19 @@ public override async Task Json_predicate_on_byte(bool async)
 """);
     }
 
+    public override async Task Json_predicate_on_byte_array(bool async)
+    {
+        await base.Json_predicate_on_byte_array(async);
+
+        AssertSql(
+            """
+SELECT [j].[Id], [j].[TestBooleanCollection], [j].[TestBooleanCollectionCollection], [j].[TestByteCollection], [j].[TestCharacterCollection], [j].[TestCharacterCollectionCollection], [j].[TestDateTimeCollection], [j].[TestDateTimeOffsetCollection], [j].[TestDecimalCollection], [j].[TestDefaultStringCollection], [j].[TestDefaultStringCollectionCollection], [j].[TestDoubleCollection], [j].[TestDoubleCollectionCollection], [j].[TestEnumCollection], [j].[TestEnumWithIntConverterCollection], [j].[TestGuidCollection], [j].[TestInt16Collection], [j].[TestInt16CollectionCollection], [j].[TestInt32Collection], [j].[TestInt32CollectionCollection], [j].[TestInt64Collection], [j].[TestInt64CollectionCollection], [j].[TestMaxLengthStringCollection], [j].[TestMaxLengthStringCollectionCollection], [j].[TestNullableEnumCollection], [j].[TestNullableEnumCollectionCollection], [j].[TestNullableEnumWithConverterThatHandlesNullsCollection], [j].[TestNullableEnumWithIntConverterCollection], [j].[TestNullableEnumWithIntConverterCollectionCollection], [j].[TestNullableInt32Collection], [j].[TestNullableInt32CollectionCollection], [j].[TestSignedByteCollection], [j].[TestSingleCollection], [j].[TestSingleCollectionCollection], [j].[TestTimeSpanCollection], [j].[TestUnsignedInt16Collection], [j].[TestUnsignedInt32Collection], [j].[TestUnsignedInt64Collection], [j].[Collection], [j].[Reference]
+FROM [JsonEntitiesAllTypes] AS [j]
+OUTER APPLY OPENJSON([j].[Reference]) WITH ([TestByteArray] varbinary(max) '$.TestByteArray') AS [t]
+WHERE [t].[TestByteArray] <> 0x010203 OR [t].[TestByteArray] IS NULL
+""");
+    }
+
     public override async Task Json_predicate_on_character(bool async)
     {
         await base.Json_predicate_on_character(async);