Skip to content

Commit

Permalink
Change Contains over null list to return empty instead of throwing
Browse files Browse the repository at this point in the history
  • Loading branch information
roji committed Apr 21, 2023
1 parent a151084 commit 191dc68
Show file tree
Hide file tree
Showing 11 changed files with 49 additions and 76 deletions.
2 changes: 1 addition & 1 deletion src/EFCore.Relational/Query/SqlNullabilityProcessor.cs
Original file line number Diff line number Diff line change
Expand Up @@ -762,7 +762,7 @@ protected virtual SqlExpression VisitIn(InExpression inExpression, bool allowOpt
case SqlParameterExpression sqlParameter:
DoNotCache();
typeMapping = sqlParameter.TypeMapping;
values = (IEnumerable?)ParameterValues[sqlParameter.Name] ?? throw new NullReferenceException();
values = (IEnumerable?)ParameterValues[sqlParameter.Name] ?? Array.Empty<object>();
break;

default:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,7 @@ public virtual SqlServerDbContextOptionsBuilder EnableRetryOnFailure(

/// <summary>
/// Sets the SQL Server compatibility level that EF Core will use when interacting with the database. This allows configuring EF
/// Core to work with older (or newer) versions of SQL Server. Defaults to <c>150</c> (SQL Server 2019).
/// Core to work with older (or newer) versions of SQL Server. Defaults to <c>160</c> (SQL Server 2022).
/// </summary>
/// <remarks>
/// See <see href="https://aka.ms/efcore-docs-dbcontext-options">Using DbContextOptions</see>, and
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ public CollectionToJsonStringConverter(Type modelClrType, CoreTypeMapping elemen

// TODO: Value converters on the element type mapping should be supported
// TODO: Full sanitization/nullability
ConvertToProvider = x => x is null ? throw new NullReferenceException() : JsonSerializer.Serialize(x);
ConvertToProvider = x => x is null ? "[]" : JsonSerializer.Serialize(x);
ConvertFromProvider = o
=> o is string s
? JsonSerializer.Deserialize(s, modelClrType)!
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -232,12 +232,10 @@ public virtual async Task Parameter_collection_null_Contains(bool async)
{
int[] ints = null;

await Assert.ThrowsAsync<NullReferenceException>(
() =>
AssertQuery(
async,
ss => ss.Set<PrimitiveCollectionsEntity>().Where(c => ints.Contains(c.Int)),
entryCount: 1));
await AssertQuery(
async,
ss => ss.Set<PrimitiveCollectionsEntity>().Where(c => ints.Contains(c.Int)),
ss => ss.Set<PrimitiveCollectionsEntity>().Where(c => false));
}

[ConditionalTheory]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ public PrimitiveCollectionsQueryOldSqlServerTest(PrimitiveCollectionsQueryOldSql
: base(fixture)
{
Fixture.TestSqlLoggerFactory.Clear();
//Fixture.TestSqlLoggerFactory.SetTestOutputHelper(testOutputHelper);
// Fixture.TestSqlLoggerFactory.SetTestOutputHelper(testOutputHelper);
}

public override async Task Inline_collection_of_ints_Contains(bool async)
Expand Down Expand Up @@ -255,7 +255,12 @@ public override async Task Parameter_collection_null_Contains(bool async)
{
await base.Parameter_collection_null_Contains(async);

AssertSql();
AssertSql(
"""
SELECT [p].[Id], [p].[Bool], [p].[Bools], [p].[DateTime], [p].[DateTimes], [p].[Enum], [p].[Enums], [p].[Int], [p].[Ints], [p].[NullableInt], [p].[NullableInts], [p].[String], [p].[Strings]
FROM [PrimitiveCollectionsEntity] AS [p]
WHERE 0 = 1
""");
}

public override Task Column_collection_of_ints_Contains(bool async)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ public PrimitiveCollectionsQuerySqlServerTest(PrimitiveCollectionsQuerySqlServer
: base(fixture)
{
Fixture.TestSqlLoggerFactory.Clear();
//Fixture.TestSqlLoggerFactory.SetTestOutputHelper(testOutputHelper);
// Fixture.TestSqlLoggerFactory.SetTestOutputHelper(testOutputHelper);
}

public override async Task Inline_collection_of_ints_Contains(bool async)
Expand Down Expand Up @@ -306,7 +306,15 @@ public override async Task Parameter_collection_null_Contains(bool async)
{
await base.Parameter_collection_null_Contains(async);

AssertSql();
AssertSql(
"""
SELECT [p].[Id], [p].[Bool], [p].[Bools], [p].[DateTime], [p].[DateTimes], [p].[Enum], [p].[Enums], [p].[Int], [p].[Ints], [p].[NullableInt], [p].[NullableInts], [p].[String], [p].[Strings]
FROM [PrimitiveCollectionsEntity] AS [p]
WHERE EXISTS (
SELECT 1
FROM OpenJson(N'[]') AS [i]
WHERE CAST([i].[value] AS int) = [p].[Int])
""");
}

public override async Task Column_collection_of_ints_Contains(bool async)
Expand Down
7 changes: 6 additions & 1 deletion test/EFCore.SqlServer.FunctionalTests/Query/QueryBugsTest.cs
Original file line number Diff line number Diff line change
Expand Up @@ -8890,9 +8890,14 @@ public virtual async Task Query_filter_with_contains_evaluates_correctly()

AssertSql(
"""
@__ef_filter___ids_0='[1,7]' (Size = 4000)

SELECT [e].[Id], [e].[Name]
FROM [Entities] AS [e]
WHERE [e].[Id] NOT IN (1, 7)
WHERE NOT (EXISTS (
SELECT 1
FROM OpenJson(@__ef_filter___ids_0) AS [e0]
WHERE CAST([e0].[value] AS int) = [e].[Id]))
""");
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -90,24 +90,19 @@ FROM [MethodCallFilter] AS [m]

public override void DbContext_list_is_parameterized()
{
using var context = CreateContext();
// Default value of TenantIds is null, which should cause a NullReferenceException to be thrown
Assert.Throws<NullReferenceException>(() => context.Set<ListFilter>().ToList());

context.TenantIds = new List<int>();
var query = context.Set<ListFilter>().ToList();
Assert.Empty(query);

context.TenantIds = new List<int> { 1 };
query = context.Set<ListFilter>().ToList();
Assert.Single(query);

context.TenantIds = new List<int> { 2, 3 };
query = context.Set<ListFilter>().ToList();
Assert.Equal(2, query.Count);
base.DbContext_list_is_parameterized();

AssertSql(
"""
SELECT [l].[Id], [l].[Tenant]
FROM [ListFilter] AS [l]
WHERE EXISTS (
SELECT 1
FROM OpenJson(N'[]') AS [e]
WHERE CAST([e].[value] AS int) = [l].[Tenant])
""",
//
"""
@__ef_filter__TenantIds_0='[]' (Size = 4000)

SELECT [l].[Id], [l].[Tenant]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,31 +11,4 @@ public NorthwindCompiledQuerySqliteTest(NorthwindQuerySqliteFixture<NoopModelCus
: base(fixture)
{
}

public override void Query_with_array_parameter()
{
var query = EF.CompileQuery(
(NorthwindContext context, string[] args)
=> context.Customers.Where(c => c.CustomerID == args[0]));

using (var context = CreateContext())
{
Assert.Equal(
CoreStrings.TranslationFailedWithDetails(
"DbSet<Customer>() .Where(c => c.CustomerID == __args .ElementAt(0))",
CoreStrings.QueryUnableToTranslateMethod("System.Linq.Enumerable", nameof(Enumerable.ElementAt))),
Assert.Throws<InvalidOperationException>(
() => query(context, new[] { "ALFKI" }).First().CustomerID).Message.Replace("\r", "").Replace("\n", ""));
}

using (var context = CreateContext())
{
Assert.Equal(
CoreStrings.TranslationFailedWithDetails(
"DbSet<Customer>() .Where(c => c.CustomerID == __args .ElementAt(0))",
CoreStrings.QueryUnableToTranslateMethod("System.Linq.Enumerable", nameof(Enumerable.ElementAt))),
Assert.Throws<InvalidOperationException>(
() => query(context, new[] { "ANATR" }).First().CustomerID).Message.Replace("\r", "").Replace("\n", ""));
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -306,7 +306,15 @@ public override async Task Parameter_collection_null_Contains(bool async)
{
await base.Parameter_collection_null_Contains(async);

AssertSql();
AssertSql(
"""
SELECT "p"."Id", "p"."Bool", "p"."Bools", "p"."DateTime", "p"."DateTimes", "p"."Enum", "p"."Enums", "p"."Int", "p"."Ints", "p"."NullableInt", "p"."NullableInts", "p"."String", "p"."Strings"
FROM "PrimitiveCollectionsEntity" AS "p"
WHERE EXISTS (
SELECT 1
FROM json_each('[]') AS "i"
WHERE "i"."value" = "p"."Int")
""");
}

public override async Task Column_collection_of_ints_Contains(bool async)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,25 +15,6 @@ public QueryFilterFuncletizationSqliteTest(
//Fixture.TestSqlLoggerFactory.SetTestOutputHelper(testOutputHelper);
}

public override void DbContext_list_is_parameterized()
{
using var context = CreateContext();
// Default value of TenantIds is null InExpression over null values throws
Assert.Throws<NullReferenceException>(() => context.Set<ListFilter>().ToList());

context.TenantIds = new List<int>();
var query = context.Set<ListFilter>().ToList();
Assert.Empty(query);

context.TenantIds = new List<int> { 1 };
query = context.Set<ListFilter>().ToList();
Assert.Single(query);

context.TenantIds = new List<int> { 2, 3 };
query = context.Set<ListFilter>().ToList();
Assert.Equal(2, query.Count);
}

public override void Using_multiple_entities_with_filters_reuses_parameters()
{
base.Using_multiple_entities_with_filters_reuses_parameters();
Expand Down

0 comments on commit 191dc68

Please sign in to comment.