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.

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

Fixes around identifier and type mappings for ValuesExpression #33439

Merged
merged 1 commit into from
Mar 31, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -366,10 +366,12 @@ JsonScalarExpression jsonScalar
{
var elementType = inlineQueryRootExpression.ElementType;

var rowExpressions = new List<RowValueExpression>();
var encounteredNull = false;
var intTypeMapping = _typeMappingSource.FindMapping(typeof(int), RelationalDependencies.Model);
RelationalTypeMapping? inferredTypeMaping = null;
var sqlExpressions = new SqlExpression[inlineQueryRootExpression.Values.Count];

// Do a first pass, translating the elements and inferring type mappings/nullability.
for (var i = 0; i < inlineQueryRootExpression.Values.Count; i++)
{
// Note that we specifically don't apply the default type mapping to the translation, to allow it to get inferred later based
Expand All @@ -380,48 +382,68 @@ JsonScalarExpression jsonScalar
return null;
}

// Infer the type mapping from the different inline elements, applying the type mapping of a column to constants/parameters, and
// also to the projection of the VALUES expression as a whole.
// TODO: This currently picks up the first type mapping; we can do better once we have a type compatibility chart (#15586)
// TODO: See similarity with SqlExpressionFactory.ApplyTypeMappingOnIn()
inferredTypeMaping ??= translatedValue.TypeMapping;

// TODO: Poor man's null semantics: in SqlNullabilityProcessor we don't fully handle the nullability of SelectExpression
// projections. Whether the SelectExpression's projection is nullable or not is determined here in translation, but at this
// point we don't know how to properly calculate nullability (and can't look at parameters).
// So for now, we assume the projected column is nullable if we see anything but non-null constants and non-nullable columns.
encounteredNull |=
translatedValue is not SqlConstantExpression { Value: not null } and not ColumnExpression { IsNullable: false };

rowExpressions.Add(
sqlExpressions[i] = translatedValue;
}

// Second pass: create the VALUES expression's row value expressions.
var rowExpressions = new RowValueExpression[sqlExpressions.Length];
for (var i = 0; i < sqlExpressions.Length; i++)
{
var sqlExpression = sqlExpressions[i];

rowExpressions[i] =
new RowValueExpression(
new[]
{
// Since VALUES may not guarantee row ordering, we add an _ord value by which we'll order.
_sqlExpressionFactory.Constant(i, intTypeMapping),
// Note that for the actual value, we must leave the type mapping null to allow it to get inferred later based on usage
translatedValue
}));
// If no type mapping was inferred (i.e. no column in the inline collection), it's left null, to allow it to get
// inferred later based on usage. Note that for the element in the VALUES expression, we'll also apply an explicit
// CONVERT to make sure the database gets the right type (see
// RelationalTypeMappingPostprocessor.ApplyTypeMappingsOnValuesExpression)
sqlExpression.TypeMapping is null && inferredTypeMaping is not null
? _sqlExpressionFactory.ApplyTypeMapping(sqlExpression, inferredTypeMaping)
: sqlExpression
});
}

var alias = _sqlAliasManager.GenerateTableAlias("values");
var valuesExpression = new ValuesExpression(alias, rowExpressions, new[] { ValuesOrderingColumnName, ValuesValueColumnName });

// Note: we leave the element type mapping null, to allow it to get inferred based on queryable operators composed on top.
var valueColumn = new ColumnExpression(
ValuesValueColumnName,
alias,
elementType.UnwrapNullableType(),
typeMapping: inferredTypeMaping,
nullable: encounteredNull);
var orderingColumn = new ColumnExpression(
ValuesOrderingColumnName,
alias,
typeof(int),
typeMapping: intTypeMapping,
nullable: false);

var selectExpression = new SelectExpression(
[valuesExpression],
new ColumnExpression(
ValuesValueColumnName,
alias,
elementType.UnwrapNullableType(),
typeMapping: null,
nullable: encounteredNull),
identifier: [],
valueColumn,
identifier: [(orderingColumn, orderingColumn.TypeMapping!.Comparer)],
_sqlAliasManager);

selectExpression.AppendOrdering(
new OrderingExpression(
selectExpression.CreateColumnExpression(
valuesExpression,
ValuesOrderingColumnName,
typeof(int),
intTypeMapping,
columnNullable: false),
ascending: true));
selectExpression.AppendOrdering(new OrderingExpression(orderingColumn, ascending: true));

Expression shaperExpression = new ProjectionBindingExpression(
selectExpression, new ProjectionMember(), encounteredNull ? elementType.MakeNullable() : elementType);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -176,17 +176,17 @@ protected virtual ValuesExpression ApplyTypeMappingsOnValuesExpression(ValuesExp

var value = rowValue.Values[j];

var inferredTypeMapping = inferredTypeMappings[j];
if (inferredTypeMapping is not null && value.TypeMapping is null)
if (value.TypeMapping is null
&& inferredTypeMappings[j] is RelationalTypeMapping inferredTypeMapping)
{
value = _sqlExpressionFactory.ApplyTypeMapping(value, inferredTypeMapping);
}

// We currently add explicit conversions on the first row, to ensure that the inferred types are properly typed.
// See #30605 for removing that when not needed.
if (i == 0)
{
value = new SqlUnaryExpression(ExpressionType.Convert, value, value.Type, value.TypeMapping);
}
// We currently add explicit conversions on the first row (but not to the _ord column), to ensure that the inferred types
// are properly typed. See #30605 for removing that when not needed.
if (i == 0 && j > 0 && value is not ColumnExpression)
{
value = new SqlUnaryExpression(ExpressionType.Convert, value, value.Type, value.TypeMapping);
}

newValues[j - (stripOrdering ? 1 : 0)] = value;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,10 @@
// The .NET Foundation licenses this file to you under the MIT license.

using System.Diagnostics.CodeAnalysis;
using Microsoft.EntityFrameworkCore.Internal;
using Microsoft.EntityFrameworkCore.Query.SqlExpressions;
using Microsoft.EntityFrameworkCore.SqlServer.Infrastructure.Internal;
using Microsoft.EntityFrameworkCore.SqlServer.Internal;
using Microsoft.EntityFrameworkCore.SqlServer.Metadata.Internal;
using Microsoft.EntityFrameworkCore.SqlServer.Storage.Internal;

namespace Microsoft.EntityFrameworkCore.SqlServer.Query.Internal;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,4 +43,12 @@ public override async Task Parameter_collection_in_subquery_Union_another_parame

Assert.Equal(RelationalStrings.SetOperationsRequireAtLeastOneSideWithValidTypeMapping("Union"), message);
}

public override async Task Project_inline_collection_with_Concat(bool async)
{
var message = (await Assert.ThrowsAsync<InvalidOperationException>(
() => base.Project_inline_collection_with_Concat(async))).Message;

Assert.Equal(RelationalStrings.InsufficientInformationToIdentifyElementOfCollectionJoin, message);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -928,6 +928,55 @@ public virtual Task Project_primitive_collections_element(bool async)
},
assertOrder: true);

[ConditionalTheory]
[MemberData(nameof(IsAsyncData))]
public virtual Task Project_inline_collection(bool async)
=> AssertQuery(
async,
ss => ss.Set<PrimitiveCollectionsEntity>().Select(x => new[] { x.String, "foo" }),
elementAsserter: (e, a) => AssertCollection(e, a, ordered: true),
assertOrder: true);

[ConditionalTheory]
[MemberData(nameof(IsAsyncData))]
public virtual Task Project_inline_collection_with_Union(bool async)
=> AssertQuery(
async,
ss => ss.Set<PrimitiveCollectionsEntity>()
.Select(
x => new
{
x.Id,
Values = new[] { x.String }.Union(ss.Set<PrimitiveCollectionsEntity>().OrderBy(xx => xx.Id).Select(xx => xx.String)).ToList()
})
.OrderBy(x => x.Id),
elementAsserter: (e, a) =>
{
Assert.Equal(e.Id, a.Id);
AssertCollection(e.Values, a.Values, ordered: false);
},
assertOrder: true);

[ConditionalTheory]
[MemberData(nameof(IsAsyncData))]
public virtual Task Project_inline_collection_with_Concat(bool async)
=> AssertQuery(
async,
ss => ss.Set<PrimitiveCollectionsEntity>()
.Select(
x => new
{
x.Id,
Values = new[] { x.String }.Concat(ss.Set<PrimitiveCollectionsEntity>().OrderBy(xx => xx.Id).Select(xx => xx.String)).ToList()
})
.OrderBy(x => x.Id),
elementAsserter: (e, a) =>
{
Assert.Equal(e.Id, a.Id);
AssertCollection(e.Values, a.Values, ordered: false);
},
assertOrder: true);

[ConditionalTheory] // #32208, #32215
[MemberData(nameof(IsAsyncData))]
public virtual Task Nested_contains_with_Lists_and_no_inferred_type_mapping(bool async)
Expand Down
14 changes: 13 additions & 1 deletion test/EFCore.Specification.Tests/TestUtilities/TestHelpers.cs
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

using System.Collections;
using Microsoft.EntityFrameworkCore.ChangeTracking.Internal;
using Microsoft.EntityFrameworkCore.Design.Internal;
using Microsoft.EntityFrameworkCore.Internal;
Expand Down Expand Up @@ -259,10 +260,21 @@ private static int AssertResults<T>(IList<T> expected, IList<T> actual)
{
Assert.True(
actual.Contains(expectedItem),
$"\r\nExpected item: [{expectedItem}] not found in results: [{string.Join(", ", actual.Take(10))}]...");
$"\r\nExpected item: '{Render(expectedItem)}' not found in results: '{string.Join(", ", actual.Take(10).Select(Render))}'...");
}

return actual.Count;

static object? Render(T element)
{
if (element is not string and IList list)
{
var start = '[' + string.Join(", ", list.ToList<object>().Take(4));
return list.Count > 4 ? start + "...]" : start + ']';
}

return element;
}
}

public static int AssertResults<T>(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -871,6 +871,30 @@ public virtual async Task Same_collection_with_conflicting_type_mappings_not_sup
Assert.Equal(RelationalStrings.ConflictingTypeMappingsInferredForColumn("value"), exception.Message);
}

[ConditionalFact]
public virtual async Task Infer_inline_collection_type_mapping()
{
var contextFactory = await InitializeAsync<TestContext>(
onModelCreating: mb => mb.Entity<TestEntity>(b => b.Property<DateTime>("DateTime").HasColumnType("datetime")));

await using var context = contextFactory.CreateContext();

_ = await context.Set<TestEntity>()
.Where(b => new[] { new DateTime(2020, 1, 1), EF.Property<DateTime>(b, "DateTime") }[0] == new DateTime(2020, 1, 1))
.ToArrayAsync();

AssertSql(
"""
SELECT [t].[Id], [t].[DateTime], [t].[Ints]
FROM [TestEntity] AS [t]
WHERE (
SELECT [v].[Value]
FROM (VALUES (0, CAST('2020-01-01T00:00:00.000' AS datetime)), (1, [t].[DateTime])) AS [v]([_ord], [Value])
ORDER BY [v].[_ord]
OFFSET 0 ROWS FETCH NEXT 1 ROWS ONLY) = '2020-01-01T00:00:00.000'
""");
}

#endregion Type mapping inference

[ConditionalFact]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -792,6 +792,43 @@ ORDER BY [p].[Id]
""");
}

public override async Task Project_inline_collection(bool async)
{
await base.Project_inline_collection(async);

AssertSql(
"""
SELECT [p].[String]
FROM [PrimitiveCollectionsEntity] AS [p]
""");
}

public override async Task Project_inline_collection_with_Union(bool async)
{
await base.Project_inline_collection_with_Union(async);

AssertSql(
"""
SELECT [p].[Id], [u].[Value]
FROM [PrimitiveCollectionsEntity] AS [p]
OUTER APPLY (
SELECT [v].[Value]
FROM (VALUES ([p].[String])) AS [v]([Value])
UNION
SELECT [p0].[String] AS [Value]
FROM [PrimitiveCollectionsEntity] AS [p0]
) AS [u]
ORDER BY [p].[Id]
""");
}

public override async Task Project_inline_collection_with_Concat(bool async)
{
await base.Project_inline_collection_with_Concat(async);

AssertSql();
}

public override async Task Nested_contains_with_Lists_and_no_inferred_type_mapping(bool async)
{
await base.Nested_contains_with_Lists_and_no_inferred_type_mapping(async);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1470,6 +1470,43 @@ ORDER BY [p].[Id]
""");
}

public override async Task Project_inline_collection(bool async)
{
await base.Project_inline_collection(async);

AssertSql(
"""
SELECT [p].[String]
FROM [PrimitiveCollectionsEntity] AS [p]
""");
}

public override async Task Project_inline_collection_with_Union(bool async)
{
await base.Project_inline_collection_with_Union(async);

AssertSql(
"""
SELECT [p].[Id], [u].[Value]
FROM [PrimitiveCollectionsEntity] AS [p]
OUTER APPLY (
SELECT [v].[Value]
FROM (VALUES ([p].[String])) AS [v]([Value])
UNION
SELECT [p0].[String] AS [Value]
FROM [PrimitiveCollectionsEntity] AS [p0]
) AS [u]
ORDER BY [p].[Id]
""");
}

public override async Task Project_inline_collection_with_Concat(bool async)
{
await base.Project_inline_collection_with_Concat(async);

AssertSql();
}

public override async Task Nested_contains_with_Lists_and_no_inferred_type_mapping(bool async)
{
await base.Nested_contains_with_Lists_and_no_inferred_type_mapping(async);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1333,6 +1333,30 @@ ORDER BY "p"."Id"
""");
}

public override async Task Project_inline_collection(bool async)
{
await base.Project_inline_collection(async);

AssertSql(
"""
SELECT "p"."String"
FROM "PrimitiveCollectionsEntity" AS "p"
""");
}

public override async Task Project_inline_collection_with_Union(bool async)
=> Assert.Equal(
SqliteStrings.ApplyNotSupported,
(await Assert.ThrowsAsync<InvalidOperationException>(
() => base.Project_inline_collection_with_Union(async))).Message);

public override async Task Project_inline_collection_with_Concat(bool async)
{
await base.Project_inline_collection_with_Concat(async);

AssertSql();
}

public override async Task Project_empty_collection_of_nullables_and_collection_only_containing_nulls(bool async)
=> Assert.Equal(
SqliteStrings.ApplyNotSupported,
Expand Down
Loading