Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Transform JSON_VALUE() to OPENJSON/WITH for binary/varbinary #33440

Merged
merged 1 commit into from
Apr 16, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions EFCore.sln.DotSettings
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand Down Expand Up @@ -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>
Expand Down
136 changes: 87 additions & 49 deletions src/EFCore.SqlServer/Query/Internal/SqlServerJsonPostprocessor.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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();

Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -198,6 +211,31 @@ when _columnsToRewrite.TryGetValue((columnExpression.TableAlias, columnExpressio
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);
}
Expand Down Expand Up @@ -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);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ public SqlServerQueryTranslationPostprocessor(
: base(dependencies, relationalDependencies, queryCompilationContext)
{
_jsonPostprocessor = new SqlServerJsonPostprocessor(
relationalDependencies.TypeMappingSource, relationalDependencies.SqlExpressionFactory);
relationalDependencies.TypeMappingSource, relationalDependencies.SqlExpressionFactory, queryCompilationContext.SqlAliasManager);
}

/// <summary>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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; }
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2297,6 +2297,19 @@ WHERE CAST(JSON_VALUE([j].[Reference], '$.TestByte') AS tinyint) <> CAST(3 AS ti
""");
}

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);
Expand Down
Loading
Loading