Skip to content

Commit 1d69411

Browse files
maumarmaumar
authored
Fix to #28881 - Consider removing unnecessary CASTs around JSON_VALUE (#29417)
We can remove cast for string scalars, other types maybe cause conversion issues in the generated sql. Fixes #28881
1 parent c771d25 commit 1d69411

File tree

7 files changed

+628
-29
lines changed

7 files changed

+628
-29
lines changed

‎src/EFCore.SqlServer/Query/Internal/SqlServerQuerySqlGenerator.cs‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -313,14 +313,17 @@ protected override Expression VisitJsonScalar(JsonScalarExpression jsonScalarExp
313313
}
314314
else
315315
{
316-
Sql.Append("CAST(JSON_VALUE(");
316+
// JSON_VALUE always returns nvarchar(4000) (https://learn.microsoft.com/sql/t-sql/functions/json-value-transact-sql),
317+
// so we cast the result to the expected type - except if it's a string (since the cast interferes with indexes over
318+
// the JSON property).
319+
Sql.Append(jsonScalarExpression.TypeMapping is StringTypeMapping ? "JSON_VALUE(" : "CAST(JSON_VALUE(");
317320
}
318321

319322
Visit(jsonScalarExpression.JsonColumn);
320323

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

323-
if (jsonScalarExpression.Type != typeof(JsonElement))
326+
if (jsonScalarExpression.TypeMapping is not SqlServerJsonTypeMapping and not StringTypeMapping)
324327
{
325328
Sql.Append(" AS ");
326329
Sql.Append(jsonScalarExpression.TypeMapping!.StoreType);

‎test/EFCore.Relational.Specification.Tests/Query/JsonQueryFixtureBase.cs‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -331,6 +331,8 @@ public static void AssertCustomNameBranch(JsonOwnedCustomNameBranch expected, Js
331331

332332
public static void AssertAllTypes(JsonOwnedAllTypes expected, JsonOwnedAllTypes actual)
333333
{
334+
Assert.Equal(expected.TestDefaultString, actual.TestDefaultString);
335+
Assert.Equal(expected.TestMaxLengthString, actual.TestMaxLengthString);
334336
Assert.Equal(expected.TestBoolean, actual.TestBoolean);
335337
Assert.Equal(expected.TestCharacter, actual.TestCharacter);
336338
Assert.Equal(expected.TestDateTime, actual.TestDateTime);
@@ -502,6 +504,7 @@ protected override void OnModelCreating(ModelBuilder modelBuilder, DbContext con
502504
x => x.Reference, b =>
503505
{
504506
b.ToJson();
507+
b.Property(x => x.TestMaxLengthString).HasMaxLength(5);
505508
b.Property(x => x.TestDecimal).HasPrecision(18, 3);
506509
b.Property(x => x.TestEnumWithIntConverter).HasConversion<int>();
507510
b.Property(x => x.TestNullableEnumWithIntConverter).HasConversion<int>();
@@ -529,6 +532,7 @@ protected override void OnModelCreating(ModelBuilder modelBuilder, DbContext con
529532
x => x.Collection, b =>
530533
{
531534
b.ToJson();
535+
b.Property(x => x.TestMaxLengthString).HasMaxLength(5);
532536
b.Property(x => x.TestDecimal).HasPrecision(18, 3);
533537
});
534538
}

‎test/EFCore.Relational.Specification.Tests/Query/JsonQueryTestBase.cs‎

Lines changed: 234 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -779,6 +779,8 @@ public virtual Task Json_all_types_projection_individual_properties(bool async)
779779
ss => ss.Set<JsonEntityAllTypes>().Select(
780780
x => new
781781
{
782+
x.Reference.TestDefaultString,
783+
x.Reference.TestMaxLengthString,
782784
x.Reference.TestBoolean,
783785
x.Reference.TestByte,
784786
x.Reference.TestCharacter,
@@ -796,7 +798,6 @@ public virtual Task Json_all_types_projection_individual_properties(bool async)
796798
x.Reference.TestUnsignedInt16,
797799
x.Reference.TestUnsignedInt32,
798800
x.Reference.TestUnsignedInt64,
799-
x.Reference.TestNullableInt32,
800801
x.Reference.TestEnum,
801802
x.Reference.TestEnumWithIntConverter,
802803
x.Reference.TestNullableEnum,
@@ -834,6 +835,238 @@ public virtual Task Json_boolean_projection_negated(bool async)
834835
async,
835836
ss => ss.Set<JsonEntityAllTypes>().Select(x => !x.Reference.TestBoolean));
836837

838+
[ConditionalTheory]
839+
[MemberData(nameof(IsAsyncData))]
840+
public virtual Task Json_predicate_on_default_string(bool async)
841+
=> AssertQuery(
842+
async,
843+
ss => ss.Set<JsonEntityAllTypes>().Where(x => x.Reference.TestDefaultString != "MyDefaultStringInReference1"),
844+
entryCount: 3);
845+
846+
[ConditionalTheory]
847+
[MemberData(nameof(IsAsyncData))]
848+
public virtual Task Json_predicate_on_max_length_string(bool async)
849+
=> AssertQuery(
850+
async,
851+
ss => ss.Set<JsonEntityAllTypes>().Where(x => x.Reference.TestMaxLengthString != "Foo"),
852+
entryCount: 3);
853+
854+
[ConditionalTheory]
855+
[MemberData(nameof(IsAsyncData))]
856+
public virtual Task Json_predicate_on_string_condition(bool async)
857+
=> AssertQuery(
858+
async,
859+
ss => ss.Set<JsonEntityAllTypes>().Where(x => (!x.Reference.TestBoolean ? x.Reference.TestMaxLengthString : x.Reference.TestDefaultString) == "MyDefaultStringInReference1"),
860+
entryCount: 3);
861+
862+
[ConditionalTheory]
863+
[MemberData(nameof(IsAsyncData))]
864+
public virtual Task Json_predicate_on_byte(bool async)
865+
=> AssertQuery(
866+
async,
867+
ss => ss.Set<JsonEntityAllTypes>().Where(x => x.Reference.TestByte != 3),
868+
entryCount: 6);
869+
870+
[ConditionalTheory]
871+
[MemberData(nameof(IsAsyncData))]
872+
public virtual Task Json_predicate_on_character(bool async)
873+
=> AssertQuery(
874+
async,
875+
ss => ss.Set<JsonEntityAllTypes>().Where(x => x.Reference.TestCharacter != 'z'),
876+
entryCount: 6);
877+
878+
[ConditionalTheory]
879+
[MemberData(nameof(IsAsyncData))]
880+
public virtual Task Json_predicate_on_datetime(bool async)
881+
=> AssertQuery(
882+
async,
883+
ss => ss.Set<JsonEntityAllTypes>().Where(x => x.Reference.TestDateTime != new DateTime(2000, 1, 3)),
884+
entryCount: 6);
885+
886+
[ConditionalTheory]
887+
[MemberData(nameof(IsAsyncData))]
888+
public virtual Task Json_predicate_on_datetimeoffset(bool async)
889+
=> AssertQuery(
890+
async,
891+
ss => ss.Set<JsonEntityAllTypes>().Where(x => x.Reference.TestDateTimeOffset != new DateTimeOffset(new DateTime(2000, 1, 4), new TimeSpan(3, 2, 0))),
892+
entryCount: 6);
893+
894+
[ConditionalTheory]
895+
[MemberData(nameof(IsAsyncData))]
896+
public virtual Task Json_predicate_on_decimal(bool async)
897+
=> AssertQuery(
898+
async,
899+
ss => ss.Set<JsonEntityAllTypes>().Where(x => x.Reference.TestDecimal != 1.35M),
900+
entryCount: 6);
901+
902+
[ConditionalTheory]
903+
[MemberData(nameof(IsAsyncData))]
904+
public virtual Task Json_predicate_on_double(bool async)
905+
=> AssertQuery(
906+
async,
907+
ss => ss.Set<JsonEntityAllTypes>().Where(x => x.Reference.TestDouble != 33.25),
908+
entryCount: 6);
909+
910+
[ConditionalTheory]
911+
[MemberData(nameof(IsAsyncData))]
912+
public virtual Task Json_predicate_on_guid(bool async)
913+
=> AssertQuery(
914+
async,
915+
ss => ss.Set<JsonEntityAllTypes>().Where(x => x.Reference.TestGuid != new Guid()),
916+
entryCount: 6);
917+
918+
[ConditionalTheory]
919+
[MemberData(nameof(IsAsyncData))]
920+
public virtual Task Json_predicate_on_int16(bool async)
921+
=> AssertQuery(
922+
async,
923+
ss => ss.Set<JsonEntityAllTypes>().Where(x => x.Reference.TestInt16 != 3),
924+
entryCount: 6);
925+
926+
[ConditionalTheory]
927+
[MemberData(nameof(IsAsyncData))]
928+
public virtual Task Json_predicate_on_int32(bool async)
929+
=> AssertQuery(
930+
async,
931+
ss => ss.Set<JsonEntityAllTypes>().Where(x => x.Reference.TestInt32 != 33),
932+
entryCount: 6);
933+
934+
[ConditionalTheory]
935+
[MemberData(nameof(IsAsyncData))]
936+
public virtual Task Json_predicate_on_int64(bool async)
937+
=> AssertQuery(
938+
async,
939+
ss => ss.Set<JsonEntityAllTypes>().Where(x => x.Reference.TestInt64 != 333),
940+
entryCount: 6);
941+
942+
[ConditionalTheory]
943+
[MemberData(nameof(IsAsyncData))]
944+
public virtual Task Json_predicate_on_signedbyte(bool async)
945+
=> AssertQuery(
946+
async,
947+
ss => ss.Set<JsonEntityAllTypes>().Where(x => x.Reference.TestSignedByte != 100),
948+
entryCount: 6);
949+
950+
[ConditionalTheory]
951+
[MemberData(nameof(IsAsyncData))]
952+
public virtual Task Json_predicate_on_single(bool async)
953+
=> AssertQuery(
954+
async,
955+
ss => ss.Set<JsonEntityAllTypes>().Where(x => x.Reference.TestSingle != 10.4f),
956+
entryCount: 6);
957+
958+
[ConditionalTheory]
959+
[MemberData(nameof(IsAsyncData))]
960+
public virtual Task Json_predicate_on_timespan(bool async)
961+
=> AssertQuery(
962+
async,
963+
ss => ss.Set<JsonEntityAllTypes>().Where(x => x.Reference.TestTimeSpan != new TimeSpan(3, 2, 0)),
964+
entryCount: 6);
965+
966+
[ConditionalTheory]
967+
[MemberData(nameof(IsAsyncData))]
968+
public virtual Task Json_predicate_on_unisgnedint16(bool async)
969+
=> AssertQuery(
970+
async,
971+
ss => ss.Set<JsonEntityAllTypes>().Where(x => x.Reference.TestUnsignedInt16 != 100),
972+
entryCount: 6);
973+
974+
[ConditionalTheory]
975+
[MemberData(nameof(IsAsyncData))]
976+
public virtual Task Json_predicate_on_unsignedint32(bool async)
977+
=> AssertQuery(
978+
async,
979+
ss => ss.Set<JsonEntityAllTypes>().Where(x => x.Reference.TestUnsignedInt32 != 1000),
980+
entryCount: 6);
981+
982+
[ConditionalTheory]
983+
[MemberData(nameof(IsAsyncData))]
984+
public virtual Task Json_predicate_on_unsignedint64(bool async)
985+
=> AssertQuery(
986+
async,
987+
ss => ss.Set<JsonEntityAllTypes>().Where(x => x.Reference.TestUnsignedInt64 != 10000),
988+
entryCount: 6);
989+
990+
[ConditionalTheory]
991+
[MemberData(nameof(IsAsyncData))]
992+
public virtual Task Json_predicate_on_enum(bool async)
993+
=> AssertQuery(
994+
async,
995+
ss => ss.Set<JsonEntityAllTypes>().Where(x => x.Reference.TestEnum != JsonEnum.Two),
996+
entryCount: 3);
997+
998+
[ConditionalTheory]
999+
[MemberData(nameof(IsAsyncData))]
1000+
public virtual Task Json_predicate_on_enumwithintconverter(bool async)
1001+
=> AssertQuery(
1002+
async,
1003+
ss => ss.Set<JsonEntityAllTypes>().Where(x => x.Reference.TestEnumWithIntConverter != JsonEnum.Three),
1004+
entryCount: 3);
1005+
1006+
[ConditionalTheory]
1007+
[MemberData(nameof(IsAsyncData))]
1008+
public virtual Task Json_predicate_on_nullableenum1(bool async)
1009+
=> AssertQuery(
1010+
async,
1011+
ss => ss.Set<JsonEntityAllTypes>().Where(x => x.Reference.TestNullableEnum != JsonEnum.One),
1012+
entryCount: 3);
1013+
1014+
[ConditionalTheory]
1015+
[MemberData(nameof(IsAsyncData))]
1016+
public virtual Task Json_predicate_on_nullableenum2(bool async)
1017+
=> AssertQuery(
1018+
async,
1019+
ss => ss.Set<JsonEntityAllTypes>().Where(x => x.Reference.TestNullableEnum != null),
1020+
entryCount: 3);
1021+
1022+
[ConditionalTheory]
1023+
[MemberData(nameof(IsAsyncData))]
1024+
public virtual Task Json_predicate_on_nullableenumwithconverterthathandlesnulls1(bool async)
1025+
=> AssertQuery(
1026+
async,
1027+
ss => ss.Set<JsonEntityAllTypes>().Where(x => x.Reference.TestNullableEnumWithConverterThatHandlesNulls != JsonEnum.One),
1028+
entryCount: 6);
1029+
1030+
[ConditionalTheory(Skip = "issue #29416")]
1031+
[MemberData(nameof(IsAsyncData))]
1032+
public virtual Task Json_predicate_on_nullableenumwithconverterthathandlesnulls2(bool async)
1033+
=> AssertQuery(
1034+
async,
1035+
ss => ss.Set<JsonEntityAllTypes>().Where(x => x.Reference.TestNullableEnumWithConverterThatHandlesNulls != null),
1036+
entryCount: 6);
1037+
1038+
[ConditionalTheory]
1039+
[MemberData(nameof(IsAsyncData))]
1040+
public virtual Task Json_predicate_on_nullableenumwithconverter1(bool async)
1041+
=> AssertQuery(
1042+
async,
1043+
ss => ss.Set<JsonEntityAllTypes>().Where(x => x.Reference.TestNullableEnumWithIntConverter != JsonEnum.Two),
1044+
entryCount: 3);
1045+
1046+
[ConditionalTheory]
1047+
[MemberData(nameof(IsAsyncData))]
1048+
public virtual Task Json_predicate_on_nullableenumwithconverter2(bool async)
1049+
=> AssertQuery(
1050+
async,
1051+
ss => ss.Set<JsonEntityAllTypes>().Where(x => x.Reference.TestNullableEnumWithIntConverter != null),
1052+
entryCount: 3);
1053+
1054+
[ConditionalTheory]
1055+
[MemberData(nameof(IsAsyncData))]
1056+
public virtual Task Json_predicate_on_nullableint321(bool async)
1057+
=> AssertQuery(
1058+
async,
1059+
ss => ss.Set<JsonEntityAllTypes>().Where(x => x.Reference.TestNullableInt32 != 100),
1060+
entryCount: 6);
1061+
1062+
[ConditionalTheory]
1063+
[MemberData(nameof(IsAsyncData))]
1064+
public virtual Task Json_predicate_on_nullableint322(bool async)
1065+
=> AssertQuery(
1066+
async,
1067+
ss => ss.Set<JsonEntityAllTypes>().Where(x => x.Reference.TestNullableInt32 != null),
1068+
entryCount: 3);
1069+
8371070
[ConditionalTheory]
8381071
[MemberData(nameof(IsAsyncData))]
8391072
public virtual Task FromSql_on_entity_with_json_basic(bool async)

‎test/EFCore.Relational.Specification.Tests/TestModels/JsonQuery/JsonOwnedAllTypes.cs‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,8 @@ namespace Microsoft.EntityFrameworkCore.TestModels.JsonQuery;
55

66
public class JsonOwnedAllTypes
77
{
8+
public string TestDefaultString { get; set; }
9+
public string TestMaxLengthString { get; set; }
810
public short TestInt16 { get; set; }
911
public int TestInt32 { get; set; }
1012
public long TestInt64 { get; set; }

‎test/EFCore.Relational.Specification.Tests/TestModels/JsonQuery/JsonQueryData.cs‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -632,6 +632,8 @@ public static IReadOnlyList<JsonEntityAllTypes> CreateJsonEntitiesAllTypes()
632632
{
633633
var r1 = new JsonOwnedAllTypes
634634
{
635+
TestDefaultString = "MyDefaultStringInReference1",
636+
TestMaxLengthString = "Foo",
635637
TestInt16 = -1234,
636638
TestInt32 = -123456789,
637639
TestInt64 = -1234567890123456789L,
@@ -659,6 +661,8 @@ public static IReadOnlyList<JsonEntityAllTypes> CreateJsonEntitiesAllTypes()
659661

660662
var r2 = new JsonOwnedAllTypes
661663
{
664+
TestDefaultString = "MyDefaultStringInReference2",
665+
TestMaxLengthString = "Bar",
662666
TestInt16 = -123,
663667
TestInt32 = -12356789,
664668
TestInt64 = -123567890123456789L,
@@ -686,6 +690,8 @@ public static IReadOnlyList<JsonEntityAllTypes> CreateJsonEntitiesAllTypes()
686690

687691
var c1 = new JsonOwnedAllTypes
688692
{
693+
TestDefaultString = "MyDefaultStringInCollection1",
694+
TestMaxLengthString = "Baz",
689695
TestInt16 = -12,
690696
TestInt32 = -12345,
691697
TestInt64 = -1234567890L,
@@ -713,6 +719,8 @@ public static IReadOnlyList<JsonEntityAllTypes> CreateJsonEntitiesAllTypes()
713719

714720
var c2 = new JsonOwnedAllTypes
715721
{
722+
TestDefaultString = "MyDefaultStringInCollection2",
723+
TestMaxLengthString = "Qux",
716724
TestInt16 = -1,
717725
TestInt32 = -1245,
718726
TestInt64 = -123567890L,

0 commit comments

Comments
 (0)