Skip to content

Commit

Permalink
Fix to #28881 - Consider removing unnecessary CASTs around JSON_VALUE
Browse files Browse the repository at this point in the history
We can remove cast for string scalars, other types maybe cause conversion issues in the generated sql.

Fixes #28881
  • Loading branch information
maumar committed Oct 25, 2022
1 parent 0a1fd5f commit f78fcf0
Show file tree
Hide file tree
Showing 3 changed files with 572 additions and 27 deletions.
12 changes: 10 additions & 2 deletions src/EFCore.SqlServer/Query/Internal/SqlServerQuerySqlGenerator.cs
Original file line number Diff line number Diff line change
Expand Up @@ -313,14 +313,22 @@ protected override Expression VisitJsonScalar(JsonScalarExpression jsonScalarExp
}
else
{
Sql.Append("CAST(JSON_VALUE(");
if (jsonScalarExpression.TypeMapping is not StringTypeMapping)
{
Sql.Append("CAST(JSON_VALUE(");
}
else
{
Sql.Append("JSON_VALUE(");
}
}

Visit(jsonScalarExpression.JsonColumn);

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

if (jsonScalarExpression.Type != typeof(JsonElement))
if (jsonScalarExpression.Type != typeof(JsonElement)
&& jsonScalarExpression.TypeMapping is not StringTypeMapping)
{
Sql.Append(" AS ");
Sql.Append(jsonScalarExpression.TypeMapping!.StoreType);
Expand Down
217 changes: 216 additions & 1 deletion test/EFCore.Relational.Specification.Tests/Query/JsonQueryTestBase.cs
Original file line number Diff line number Diff line change
Expand Up @@ -796,14 +796,229 @@ public virtual Task Json_all_types_projection_individual_properties(bool async)
x.Reference.TestUnsignedInt16,
x.Reference.TestUnsignedInt32,
x.Reference.TestUnsignedInt64,
x.Reference.TestNullableInt32,
x.Reference.TestEnum,
x.Reference.TestEnumWithIntConverter,
x.Reference.TestNullableEnum,
x.Reference.TestNullableEnumWithIntConverter,
x.Reference.TestNullableEnumWithConverterThatHandlesNulls,
}));

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

[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);

[ConditionalTheory]
[MemberData(nameof(IsAsyncData))]
public virtual Task FromSql_on_entity_with_json_basic(bool async)
Expand Down
Loading

0 comments on commit f78fcf0

Please sign in to comment.