Skip to content

Commit

Permalink
[release/7.0] Fix to #28881 - Consider removing unnecessary CASTs aro…
Browse files Browse the repository at this point in the history 
…und JSON_VALUE

We can remove cast for string scalars, other types maybe cause conversion issues in the generated sql.

Fixes #28881
  • Loading branch information
@maumar
maumar committed Mar 9, 2023
1 parent 85d686a commit b398fd9
Show file tree
Hide file tree
Showing 6 changed files with 628 additions and 31 deletions.
36 changes: 31 additions & 5 deletions src/EFCore.SqlServer/Query/Internal/SqlServerQuerySqlGenerator.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,9 @@ namespace Microsoft.EntityFrameworkCore.SqlServer.Query.Internal;
/// </summary>
public class SqlServerQuerySqlGenerator : QuerySqlGenerator
{
private static readonly bool UseOldBehavior28881
= AppContext.TryGetSwitch("Microsoft.EntityFrameworkCore.Issue28881", out var enabled28881) && enabled28881;

private static readonly bool UseOldBehavior29667
= AppContext.TryGetSwitch("Microsoft.EntityFrameworkCore.Issue29667", out var enabled29667) && enabled29667;

Expand Down Expand Up @@ -323,18 +326,41 @@ protected override Expression VisitJsonScalar(JsonScalarExpression jsonScalarExp
}
else
{
Sql.Append("CAST(JSON_VALUE(");
if (!UseOldBehavior28881)
{
// JSON_VALUE always returns nvarchar(4000) (https://learn.microsoft.com/sql/t-sql/functions/json-value-transact-sql),
// so we cast the result to the expected type - except if it's a string (since the cast interferes with indexes over
// the JSON property).
Sql.Append(jsonScalarExpression.TypeMapping is StringTypeMapping ? "JSON_VALUE(" : "CAST(JSON_VALUE(");
}
else
{
Sql.Append("CAST(JSON_VALUE(");
}
}

Visit(jsonScalarExpression.JsonColumn);

Sql.Append($",'{string.Join("", jsonScalarExpression.Path.Select(e => e.ToString()))}')");

if (jsonScalarExpression.Type != typeof(JsonElement))

if (!UseOldBehavior28881)
{
Sql.Append(" AS ");
Sql.Append(jsonScalarExpression.TypeMapping!.StoreType);
Sql.Append(")");
if (jsonScalarExpression.TypeMapping is not SqlServerJsonTypeMapping and not StringTypeMapping)
{
Sql.Append(" AS ");
Sql.Append(jsonScalarExpression.TypeMapping!.StoreType);
Sql.Append(")");
}
}
else
{
if (jsonScalarExpression.Type != typeof(JsonElement))
{
Sql.Append(" AS ");
Sql.Append(jsonScalarExpression.TypeMapping!.StoreType);
Sql.Append(")");
}
}

return jsonScalarExpression;
Expand Down
226 changes: 226 additions & 0 deletions test/EFCore.Relational.Specification.Tests/Query/JsonQueryTestBase.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -779,6 +779,8 @@ public virtual Task Json_all_types_projection_individual_properties(bool async)
ss => ss.Set<JsonEntityAllTypes>().Select(
x => new
{
x.Reference.TestDefaultString,
x.Reference.TestMaxLengthString,
x.Reference.TestBoolean,
x.Reference.TestByte,
x.Reference.TestCharacter,
Expand All @@ -803,4 +805,228 @@ public virtual Task Json_all_types_projection_individual_properties(bool async)
x.Reference.TestNullableEnumWithIntConverter,
x.Reference.TestNullableEnumWithConverterThatHandlesNulls,
}));

[ConditionalTheory]
[MemberData(nameof(IsAsyncData))]
public virtual Task Json_predicate_on_default_string(bool async)
=> AssertQuery(
async,
ss => ss.Set<JsonEntityAllTypes>().Where(x => x.Reference.TestDefaultString != "MyDefaultStringInReference1"),
entryCount: 3);

[ConditionalTheory]
[MemberData(nameof(IsAsyncData))]
public virtual Task Json_predicate_on_max_length_string(bool async)
=> AssertQuery(
async,
ss => ss.Set<JsonEntityAllTypes>().Where(x => x.Reference.TestMaxLengthString != "Foo"),
entryCount: 3);

[ConditionalTheory]
[MemberData(nameof(IsAsyncData))]
public virtual Task Json_predicate_on_byte(bool async)
=> AssertQuery(
async,
ss => ss.Set<JsonEntityAllTypes>().Where(x => x.Reference.TestByte != 3),
entryCount: 6);

[ConditionalTheory]
[MemberData(nameof(IsAsyncData))]
public virtual Task Json_predicate_on_character(bool async)
=> AssertQuery(
async,
ss => ss.Set<JsonEntityAllTypes>().Where(x => x.Reference.TestCharacter != 'z'),
entryCount: 6);

[ConditionalTheory]
[MemberData(nameof(IsAsyncData))]
public virtual Task Json_predicate_on_datetime(bool async)
=> AssertQuery(
async,
ss => ss.Set<JsonEntityAllTypes>().Where(x => x.Reference.TestDateTime != new DateTime(2000, 1, 3)),
entryCount: 6);

[ConditionalTheory]
[MemberData(nameof(IsAsyncData))]
public virtual Task Json_predicate_on_datetimeoffset(bool async)
=> AssertQuery(
async,
ss => ss.Set<JsonEntityAllTypes>().Where(x => x.Reference.TestDateTimeOffset != new DateTimeOffset(new DateTime(2000, 1, 4), new TimeSpan(3, 2, 0))),
entryCount: 6);

[ConditionalTheory]
[MemberData(nameof(IsAsyncData))]
public virtual Task Json_predicate_on_decimal(bool async)
=> AssertQuery(
async,
ss => ss.Set<JsonEntityAllTypes>().Where(x => x.Reference.TestDecimal != 1.35M),
entryCount: 6);

[ConditionalTheory]
[MemberData(nameof(IsAsyncData))]
public virtual Task Json_predicate_on_double(bool async)
=> AssertQuery(
async,
ss => ss.Set<JsonEntityAllTypes>().Where(x => x.Reference.TestDouble != 33.25),
entryCount: 6);

[ConditionalTheory]
[MemberData(nameof(IsAsyncData))]
public virtual Task Json_predicate_on_guid(bool async)
=> AssertQuery(
async,
ss => ss.Set<JsonEntityAllTypes>().Where(x => x.Reference.TestGuid != new Guid()),
entryCount: 6);

[ConditionalTheory]
[MemberData(nameof(IsAsyncData))]
public virtual Task Json_predicate_on_int16(bool async)
=> AssertQuery(
async,
ss => ss.Set<JsonEntityAllTypes>().Where(x => x.Reference.TestInt16 != 3),
entryCount: 6);

[ConditionalTheory]
[MemberData(nameof(IsAsyncData))]
public virtual Task Json_predicate_on_int32(bool async)
=> AssertQuery(
async,
ss => ss.Set<JsonEntityAllTypes>().Where(x => x.Reference.TestInt32 != 33),
entryCount: 6);

[ConditionalTheory]
[MemberData(nameof(IsAsyncData))]
public virtual Task Json_predicate_on_int64(bool async)
=> AssertQuery(
async,
ss => ss.Set<JsonEntityAllTypes>().Where(x => x.Reference.TestInt64 != 333),
entryCount: 6);

[ConditionalTheory]
[MemberData(nameof(IsAsyncData))]
public virtual Task Json_predicate_on_signedbyte(bool async)
=> AssertQuery(
async,
ss => ss.Set<JsonEntityAllTypes>().Where(x => x.Reference.TestSignedByte != 100),
entryCount: 6);

[ConditionalTheory]
[MemberData(nameof(IsAsyncData))]
public virtual Task Json_predicate_on_single(bool async)
=> AssertQuery(
async,
ss => ss.Set<JsonEntityAllTypes>().Where(x => x.Reference.TestSingle != 10.4f),
entryCount: 6);

[ConditionalTheory]
[MemberData(nameof(IsAsyncData))]
public virtual Task Json_predicate_on_timespan(bool async)
=> AssertQuery(
async,
ss => ss.Set<JsonEntityAllTypes>().Where(x => x.Reference.TestTimeSpan != new TimeSpan(3, 2, 0)),
entryCount: 6);

[ConditionalTheory]
[MemberData(nameof(IsAsyncData))]
public virtual Task Json_predicate_on_unisgnedint16(bool async)
=> AssertQuery(
async,
ss => ss.Set<JsonEntityAllTypes>().Where(x => x.Reference.TestUnsignedInt16 != 100),
entryCount: 6);

[ConditionalTheory]
[MemberData(nameof(IsAsyncData))]
public virtual Task Json_predicate_on_unsignedint32(bool async)
=> AssertQuery(
async,
ss => ss.Set<JsonEntityAllTypes>().Where(x => x.Reference.TestUnsignedInt32 != 1000),
entryCount: 6);

[ConditionalTheory]
[MemberData(nameof(IsAsyncData))]
public virtual Task Json_predicate_on_unsignedint64(bool async)
=> AssertQuery(
async,
ss => ss.Set<JsonEntityAllTypes>().Where(x => x.Reference.TestUnsignedInt64 != 10000),
entryCount: 6);

[ConditionalTheory]
[MemberData(nameof(IsAsyncData))]
public virtual Task Json_predicate_on_enum(bool async)
=> AssertQuery(
async,
ss => ss.Set<JsonEntityAllTypes>().Where(x => x.Reference.TestEnum != JsonEnum.Two),
entryCount: 3);

[ConditionalTheory]
[MemberData(nameof(IsAsyncData))]
public virtual Task Json_predicate_on_enumwithintconverter(bool async)
=> AssertQuery(
async,
ss => ss.Set<JsonEntityAllTypes>().Where(x => x.Reference.TestEnumWithIntConverter != JsonEnum.Three),
entryCount: 3);

[ConditionalTheory]
[MemberData(nameof(IsAsyncData))]
public virtual Task Json_predicate_on_nullableenum1(bool async)
=> AssertQuery(
async,
ss => ss.Set<JsonEntityAllTypes>().Where(x => x.Reference.TestNullableEnum != JsonEnum.One),
entryCount: 3);

[ConditionalTheory]
[MemberData(nameof(IsAsyncData))]
public virtual Task Json_predicate_on_nullableenum2(bool async)
=> AssertQuery(
async,
ss => ss.Set<JsonEntityAllTypes>().Where(x => x.Reference.TestNullableEnum != null),
entryCount: 3);

[ConditionalTheory]
[MemberData(nameof(IsAsyncData))]
public virtual Task Json_predicate_on_nullableenumwithconverterthathandlesnulls1(bool async)
=> AssertQuery(
async,
ss => ss.Set<JsonEntityAllTypes>().Where(x => x.Reference.TestNullableEnumWithConverterThatHandlesNulls != JsonEnum.One),
entryCount: 6);

[ConditionalTheory(Skip = "issue #29416")]
[MemberData(nameof(IsAsyncData))]
public virtual Task Json_predicate_on_nullableenumwithconverterthathandlesnulls2(bool async)
=> AssertQuery(
async,
ss => ss.Set<JsonEntityAllTypes>().Where(x => x.Reference.TestNullableEnumWithConverterThatHandlesNulls != null),
entryCount: 6);

[ConditionalTheory]
[MemberData(nameof(IsAsyncData))]
public virtual Task Json_predicate_on_nullableenumwithconverter1(bool async)
=> AssertQuery(
async,
ss => ss.Set<JsonEntityAllTypes>().Where(x => x.Reference.TestNullableEnumWithIntConverter != JsonEnum.Two),
entryCount: 3);

[ConditionalTheory]
[MemberData(nameof(IsAsyncData))]
public virtual Task Json_predicate_on_nullableenumwithconverter2(bool async)
=> AssertQuery(
async,
ss => ss.Set<JsonEntityAllTypes>().Where(x => x.Reference.TestNullableEnumWithIntConverter != null),
entryCount: 3);

[ConditionalTheory]
[MemberData(nameof(IsAsyncData))]
public virtual Task Json_predicate_on_nullableint321(bool async)
=> AssertQuery(
async,
ss => ss.Set<JsonEntityAllTypes>().Where(x => x.Reference.TestNullableInt32 != 100),
entryCount: 6);

[ConditionalTheory]
[MemberData(nameof(IsAsyncData))]
public virtual Task Json_predicate_on_nullableint322(bool async)
=> AssertQuery(
async,
ss => ss.Set<JsonEntityAllTypes>().Where(x => x.Reference.TestNullableInt32 != null),
entryCount: 3);
}
2 changes: 2 additions & 0 deletions test/EFCore.Relational.Specification.Tests/TestModels/JsonQuery/JsonOwnedAllTypes.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@ namespace Microsoft.EntityFrameworkCore.TestModels.JsonQuery;

public class JsonOwnedAllTypes
{
public string TestDefaultString { get; set; }
public string TestMaxLengthString { get; set; }
public short TestInt16 { get; set; }
public int TestInt32 { get; set; }
public long TestInt64 { get; set; }
Expand Down
8 changes: 8 additions & 0 deletions test/EFCore.Relational.Specification.Tests/TestModels/JsonQuery/JsonQueryData.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -632,6 +632,8 @@ public static IReadOnlyList<JsonEntityAllTypes> CreateJsonEntitiesAllTypes()
{
var r1 = new JsonOwnedAllTypes
{
TestDefaultString = "MyDefaultStringInReference1",
TestMaxLengthString = "Foo",
TestInt16 = -1234,
TestInt32 = -123456789,
TestInt64 = -1234567890123456789L,
Expand Down Expand Up @@ -659,6 +661,8 @@ public static IReadOnlyList<JsonEntityAllTypes> CreateJsonEntitiesAllTypes()

var r2 = new JsonOwnedAllTypes
{
TestDefaultString = "MyDefaultStringInReference2",
TestMaxLengthString = "Bar",
TestInt16 = -123,
TestInt32 = -12356789,
TestInt64 = -123567890123456789L,
Expand Down Expand Up @@ -686,6 +690,8 @@ public static IReadOnlyList<JsonEntityAllTypes> CreateJsonEntitiesAllTypes()

var c1 = new JsonOwnedAllTypes
{
TestDefaultString = "MyDefaultStringInCollection1",
TestMaxLengthString = "Baz",
TestInt16 = -12,
TestInt32 = -12345,
TestInt64 = -1234567890L,
Expand Down Expand Up @@ -713,6 +719,8 @@ public static IReadOnlyList<JsonEntityAllTypes> CreateJsonEntitiesAllTypes()

var c2 = new JsonOwnedAllTypes
{
TestDefaultString = "MyDefaultStringInCollection2",
TestMaxLengthString = "Qux",
TestInt16 = -1,
TestInt32 = -1245,
TestInt64 = -123567890L,
Expand Down
Loading

0 comments on commit b398fd9

Please sign in to comment.