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.

Sign up for GitHub

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

Jump to bottom

[release/7.0] Fix to #28881 - Consider removing unnecessary CASTs around JSON_VALUE #30442

Closed
wants to merge 1 commit into from
Closed

[release/7.0] Fix to #28881 - Consider removing unnecessary CASTs around JSON_VALUE #30442

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
36 changes: 31 additions & 5 deletions src/EFCore.SqlServer/Query/Internal/SqlServerQuerySqlGenerator.cs
View file
Open in desktop
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)
{
Sql.Append("CAST(JSON_VALUE(");
}
else
{
// 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(");
}
}

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.Type != typeof(JsonElement))
{
Sql.Append(" AS ");
Sql.Append(jsonScalarExpression.TypeMapping!.StoreType);
Sql.Append(")");
}
}
else
{
if (jsonScalarExpression.TypeMapping is not SqlServerJsonTypeMapping and not StringTypeMapping)
{
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
Open in desktop
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
Open in desktop
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
Open in desktop
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