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

Fix to #28881 - Consider removing unnecessary CASTs around JSON_VALUE #29417

Merged
merged 1 commit into from
Nov 20, 2022
Merged

Fix to #28881 - Consider removing unnecessary CASTs around JSON_VALUE #29417

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
7 changes: 5 additions & 2 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 @@ -313,14 +313,17 @@ protected override Expression VisitJsonScalar(JsonScalarExpression jsonScalarExp
}
else
{
Sql.Append("CAST(JSON_VALUE(");
// 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 (jsonScalarExpression.TypeMapping is not SqlServerJsonTypeMapping and not StringTypeMapping)
{
Sql.Append(" AS ");
Sql.Append(jsonScalarExpression.TypeMapping!.StoreType);
Expand Down
4 changes: 4 additions & 0 deletions test/EFCore.Relational.Specification.Tests/Query/JsonQueryFixtureBase.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -331,6 +331,8 @@ public static void AssertCustomNameBranch(JsonOwnedCustomNameBranch expected, Js

public static void AssertAllTypes(JsonOwnedAllTypes expected, JsonOwnedAllTypes actual)
{
Assert.Equal(expected.TestDefaultString, actual.TestDefaultString);
Assert.Equal(expected.TestMaxLengthString, actual.TestMaxLengthString);
Assert.Equal(expected.TestBoolean, actual.TestBoolean);
Assert.Equal(expected.TestCharacter, actual.TestCharacter);
Assert.Equal(expected.TestDateTime, actual.TestDateTime);
Expand Down Expand Up @@ -502,6 +504,7 @@ protected override void OnModelCreating(ModelBuilder modelBuilder, DbContext con
x => x.Reference, b =>
{
b.ToJson();
b.Property(x => x.TestMaxLengthString).HasMaxLength(5);
b.Property(x => x.TestDecimal).HasPrecision(18, 3);
b.Property(x => x.TestEnumWithIntConverter).HasConversion<int>();
b.Property(x => x.TestNullableEnumWithIntConverter).HasConversion<int>();
Expand Down Expand Up @@ -529,6 +532,7 @@ protected override void OnModelCreating(ModelBuilder modelBuilder, DbContext con
x => x.Collection, b =>
{
b.ToJson();
b.Property(x => x.TestMaxLengthString).HasMaxLength(5);
b.Property(x => x.TestDecimal).HasPrecision(18, 3);
});
}
Expand Down
235 changes: 234 additions & 1 deletion 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 @@ -796,7 +798,6 @@ 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,
Expand Down Expand Up @@ -834,6 +835,238 @@ public virtual Task Json_boolean_projection_negated(bool async)
async,
ss => ss.Set<JsonEntityAllTypes>().Select(x => !x.Reference.TestBoolean));

[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_string_condition(bool async)
=> AssertQuery(
async,
ss => ss.Set<JsonEntityAllTypes>().Where(x => (!x.Reference.TestBoolean ? x.Reference.TestMaxLengthString : x.Reference.TestDefaultString) == "MyDefaultStringInReference1"),
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);

[ConditionalTheory]
[MemberData(nameof(IsAsyncData))]
public virtual Task FromSql_on_entity_with_json_basic(bool async)
Expand Down
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