From d1428e10d52d91c51c8f8713f75fd1bae83c42f8 Mon Sep 17 00:00:00 2001 From: Shay Rojansky Date: Wed, 13 Mar 2024 01:43:05 +0100 Subject: [PATCH 1/2] Extract type mapping postprocessing to an external visitor Part of API review (#33220) --- ...RelationalQueryTranslationPostprocessor.cs | 35 +- ...eryTranslationPostprocessorDependencies.cs | 9 +- ...yableMethodTranslatingExpressionVisitor.cs | 430 +----------------- .../RelationalTypeMappingPostprocessor.cs | 413 +++++++++++++++++ .../SqlServerQueryTranslationPostprocessor.cs | 15 +- ...verQueryTranslationPostprocessorFactory.cs | 3 +- ...yableMethodTranslatingExpressionVisitor.cs | 102 ----- .../SqlServerTypeMappingPostprocessor.cs | 94 ++++ .../SqliteQueryTranslationPostprocessor.cs | 9 + ...yableMethodTranslatingExpressionVisitor.cs | 136 +----- .../SqliteTypeMappingPostprocessor.cs | 127 ++++++ .../LazyLoadProxyTestBase.cs | 2 +- 12 files changed, 719 insertions(+), 656 deletions(-) create mode 100644 src/EFCore.Relational/Query/RelationalTypeMappingPostprocessor.cs create mode 100644 src/EFCore.SqlServer/Query/Internal/SqlServerTypeMappingPostprocessor.cs create mode 100644 src/EFCore.Sqlite.Core/Query/Internal/SqliteTypeMappingPostprocessor.cs diff --git a/src/EFCore.Relational/Query/RelationalQueryTranslationPostprocessor.cs b/src/EFCore.Relational/Query/RelationalQueryTranslationPostprocessor.cs index 5f769a49f5f..04ea10c8b05 100644 --- a/src/EFCore.Relational/Query/RelationalQueryTranslationPostprocessor.cs +++ b/src/EFCore.Relational/Query/RelationalQueryTranslationPostprocessor.cs @@ -27,6 +27,7 @@ public RelationalQueryTranslationPostprocessor( : base(dependencies, queryCompilationContext) { RelationalDependencies = relationalDependencies; + RelationalQueryCompilationContext = queryCompilationContext; _sqlAliasManager = queryCompilationContext.SqlAliasManager; _useRelationalNulls = RelationalOptionsExtension.Extract(queryCompilationContext.ContextOptions).UseRelationalNulls; } @@ -36,31 +37,45 @@ public RelationalQueryTranslationPostprocessor( /// protected virtual RelationalQueryTranslationPostprocessorDependencies RelationalDependencies { get; } + /// + /// The query compilation context object for current compilation. + /// + protected virtual RelationalQueryCompilationContext RelationalQueryCompilationContext { get; } + /// public override Expression Process(Expression query) { var query1 = base.Process(query); - var query2 = new SelectExpressionProjectionApplyingExpressionVisitor( - ((RelationalQueryCompilationContext)QueryCompilationContext).QuerySplittingBehavior).Visit(query1); - var query3 = Prune(query2); + var query2 = ProcessTypeMappings(query1); + var query3 = new SelectExpressionProjectionApplyingExpressionVisitor( + ((RelationalQueryCompilationContext)QueryCompilationContext).QuerySplittingBehavior).Visit(query2); + var query4 = Prune(query3); // TODO: This - and all the verifications below - should happen after all visitors have run, including provider-specific ones. - var query4 = _sqlAliasManager.PostprocessAliases(query3); + var query5 = _sqlAliasManager.PostprocessAliases(query4); #if DEBUG // Verifies that all SelectExpression are marked as immutable after this point. - new SelectExpressionMutableVerifyingExpressionVisitor().Visit(query4); + new SelectExpressionMutableVerifyingExpressionVisitor().Visit(query5); #endif - var query5 = new SqlExpressionSimplifyingExpressionVisitor(RelationalDependencies.SqlExpressionFactory, _useRelationalNulls) - .Visit(query4); - var query6 = new RelationalValueConverterCompensatingExpressionVisitor(RelationalDependencies.SqlExpressionFactory).Visit(query5); + var query6 = new SqlExpressionSimplifyingExpressionVisitor(RelationalDependencies.SqlExpressionFactory, _useRelationalNulls) + .Visit(query5); + var query7 = new RelationalValueConverterCompensatingExpressionVisitor(RelationalDependencies.SqlExpressionFactory).Visit(query6); - return query6; + return query7; } /// - /// Prunes unnecessarily objects from the SQL tree, e.g. tables which aren't referenced by any column. + /// Performs various postprocessing related to type mappings, e.g. applies inferred type mappings for queryable constants/parameters + /// and verifies that all have a type mapping. + /// + /// The query expression to process. + protected virtual Expression ProcessTypeMappings(Expression expression) + => new RelationalTypeMappingPostprocessor(Dependencies, RelationalDependencies, RelationalQueryCompilationContext).Process(expression); + + /// + /// Prunes unnecessary objects from the SQL tree, e.g. tables which aren't referenced by any column. /// Can be overridden by providers for provider-specific pruning. /// protected virtual Expression Prune(Expression query) diff --git a/src/EFCore.Relational/Query/RelationalQueryTranslationPostprocessorDependencies.cs b/src/EFCore.Relational/Query/RelationalQueryTranslationPostprocessorDependencies.cs index 3eb8ccbf374..46dafc54d01 100644 --- a/src/EFCore.Relational/Query/RelationalQueryTranslationPostprocessorDependencies.cs +++ b/src/EFCore.Relational/Query/RelationalQueryTranslationPostprocessorDependencies.cs @@ -46,13 +46,20 @@ public sealed record RelationalQueryTranslationPostprocessorDependencies /// [EntityFrameworkInternal] public RelationalQueryTranslationPostprocessorDependencies( - ISqlExpressionFactory sqlExpressionFactory) + ISqlExpressionFactory sqlExpressionFactory, + IRelationalTypeMappingSource typeMappingSource) { SqlExpressionFactory = sqlExpressionFactory; + TypeMappingSource = typeMappingSource; } /// /// The SQL expression factory. /// public ISqlExpressionFactory SqlExpressionFactory { get; init; } + + /// + /// The SQL expression factory. + /// + public IRelationalTypeMappingSource TypeMappingSource { get; init; } } diff --git a/src/EFCore.Relational/Query/RelationalQueryableMethodTranslatingExpressionVisitor.cs b/src/EFCore.Relational/Query/RelationalQueryableMethodTranslatingExpressionVisitor.cs index ccbdb80cf56..d65226df9b3 100644 --- a/src/EFCore.Relational/Query/RelationalQueryableMethodTranslatingExpressionVisitor.cs +++ b/src/EFCore.Relational/Query/RelationalQueryableMethodTranslatingExpressionVisitor.cs @@ -2,7 +2,6 @@ // The .NET Foundation licenses this file to you under the MIT license. using System.Diagnostics.CodeAnalysis; -using Microsoft.EntityFrameworkCore.Metadata.Internal; using Microsoft.EntityFrameworkCore.Query.Internal; using Microsoft.EntityFrameworkCore.Query.SqlExpressions; @@ -12,7 +11,6 @@ namespace Microsoft.EntityFrameworkCore.Query; public partial class RelationalQueryableMethodTranslatingExpressionVisitor : QueryableMethodTranslatingExpressionVisitor { private const string SqlQuerySingleColumnAlias = "Value"; - private const string ValuesOrderingColumnName = "_ord", ValuesValueColumnName = "Value"; private readonly RelationalSqlTranslatingExpressionVisitor _sqlTranslator; private readonly SharedTypeEntityExpandingExpressionVisitor _sharedTypeEntityExpandingExpressionVisitor; @@ -23,6 +21,24 @@ public partial class RelationalQueryableMethodTranslatingExpressionVisitor : Que private readonly ISqlExpressionFactory _sqlExpressionFactory; private readonly bool _subquery; + /// + /// This is an internal API that supports the Entity Framework Core infrastructure and not subject to + /// the same compatibility standards as public APIs. It may be changed or removed without notice in + /// any release. You should only use it directly in your code with extreme caution and knowing that + /// doing so can result in application failures when updating to a new Entity Framework Core release. + /// + [EntityFrameworkInternal] + public const string ValuesOrderingColumnName = "_ord"; + + /// + /// This is an internal API that supports the Entity Framework Core infrastructure and not subject to + /// the same compatibility standards as public APIs. It may be changed or removed without notice in + /// any release. You should only use it directly in your code with extreme caution and knowing that + /// doing so can result in application failures when updating to a new Entity Framework Core release. + /// + [EntityFrameworkInternal] + public const string ValuesValueColumnName = "Value"; + /// /// Creates a new instance of the class. /// @@ -73,27 +89,6 @@ protected RelationalQueryableMethodTranslatingExpressionVisitor( _subquery = true; } - /// - public override Expression Translate(Expression expression) - { - var visited = base.Translate(expression); - - if (!_subquery) - { - // We've finished translating the entire query. - - // If any constant/parameter query roots exist in the query, their columns don't yet have a type mapping. - // First, scan the query tree for inferred type mappings (e.g. based on a comparison of those columns to some regular column - // with a type mapping). - var inferredColumns = new ColumnTypeMappingScanner().Scan(visited); - - // Then, apply those type mappings back on the constant/parameter tables (e.g. ValuesExpression). - visited = ApplyInferredTypeMappings(visited, inferredColumns); - } - - return visited; - } - /// protected override Expression VisitExtension(Expression extensionExpression) { @@ -1328,11 +1323,11 @@ protected override ShapedQueryExpression TranslateUnion(ShapedQueryExpression so /// Inferred type mappings for queryable constants/parameters collected during translation. These will be applied to the appropriate /// nodes in the tree. /// + [Obsolete("Override RelationalQueryTranslationPostprocessor.ProcessTypeMappings() instead.", error: true)] protected virtual Expression ApplyInferredTypeMappings( Expression expression, IReadOnlyDictionary<(string, string), RelationalTypeMapping?> inferredTypeMappings) - => new RelationalInferredTypeMappingApplier( - RelationalDependencies.Model, _sqlExpressionFactory, inferredTypeMappings).Visit(expression); + => throw new UnreachableException(); /// /// Determines whether the given is ordered, typically because orderings have been added to it. @@ -2102,387 +2097,8 @@ private bool TryExtractBareInlineCollectionValues(ShapedQueryExpression shapedQu } /// - /// A visitor which scans an expression tree and attempts to find columns for which we were missing type mappings (projected out - /// of queryable constant/parameter), and those type mappings have been inferred. + /// This visitor has been obsoleted; extend RelationalTypeMappingPostprocessor instead. /// - /// - /// - /// This handles two cases: (1) an untyped column which type-inferred in the regular way, e.g. through comparison to a typed - /// column, and (2) set operations where on side is typed and the other is untyped. - /// - /// - /// Note that this visitor follows type columns across subquery projections. That is, if a root constant/parameter is buried - /// within subqueries, and somewhere above the column projected out of a subquery is inferred, this is picked up and propagated - /// all the way down. - /// - /// - /// The visitor dose not change the query tree in any way - it only populates the inferred type mappings it identified in - /// the given dictionary; actual application of the inferred type mappings happens later in - /// . We can't do this in a single pass since untyped roots - /// (e.g. may get visited before the type-inferred column referring to them (e.g. CROSS APPLY, - /// correlated subquery). - /// - /// - private sealed class ColumnTypeMappingScanner : ExpressionVisitor - { - private readonly Dictionary<(string TableAlias, string ColumnName), RelationalTypeMapping?> _inferredColumns = new(); - - /// - /// A mapping of table aliases to the instances; these are used to check the table type - /// when we encounter a typed column pointing to it, and avoid recording inferred type mappings where we know the table - /// doesn't need to be inferred from the column. - /// - private readonly Dictionary _tableAliasMap = new(); - - private string? _currentSelectTableAlias; - private ProjectionExpression? _currentProjectionExpression; - - public IReadOnlyDictionary<(string, string), RelationalTypeMapping?> Scan(Expression expression) - { - _inferredColumns.Clear(); - _tableAliasMap.Clear(); - - Visit(expression); - - return _inferredColumns; - } - - protected override Expression VisitExtension(Expression node) - { - if (node is TableExpressionBase { Alias: string tableAlias } table) - { - _tableAliasMap[tableAlias] = table.UnwrapJoin(); - } - - switch (node) - { - // A column on a table which was possibly originally untyped (constant/parameter root or a subquery projection of one), - // which now does have a type mapping - this would mean in got inferred in the usual manner (comparison with typed column). - // Registered the inferred type mapping so it can be later applied back to its table, if it's untyped. - case ColumnExpression { TypeMapping: { } typeMapping } c when WasMaybeOriginallyUntyped(c): - { - RegisterInferredTypeMapping(c, typeMapping); - - return base.VisitExtension(node); - } - - // Similar to the above, but with ScalarSubqueryExpression the inferred type mapping is on the expression itself, while the - // ColumnExpression we need is on the subquery's projection. - case ScalarSubqueryExpression - { - TypeMapping: { } typeMapping, - Subquery.Projection: [{ Expression: ColumnExpression columnExpression }] - }: - { - var visitedSubquery = base.VisitExtension(node); - - if (WasMaybeOriginallyUntyped(columnExpression)) - { - RegisterInferredTypeMapping(columnExpression, typeMapping); - } - - return visitedSubquery; - } - - // InExpression over a subquery: apply the item's type mapping on the subquery - case InExpression - { - Item.TypeMapping: { } typeMapping, - Subquery.Projection: [{ Expression: ColumnExpression columnExpression }] - }: - { - var visited = base.VisitExtension(node); - - if (WasMaybeOriginallyUntyped(columnExpression)) - { - RegisterInferredTypeMapping(columnExpression, typeMapping); - } - - return visited; - } - - // For set operations involving a leg with a type mapping (e.g. some column) and a leg without one (queryable constant or - // parameter), we infer the missing type mapping from the other side. - case SetOperationBase - { - Source1.Projection: [{ Expression: var projection1 }], - Source2.Projection: [{ Expression: var projection2 }] - } - when UnwrapConvert(projection1) is ColumnExpression column1 && UnwrapConvert(projection2) is ColumnExpression column2: - { - // Note that we can't use WasMaybeOriginallyUntyped() here like in the other cases, since that only works after we've - // visited the table the column points to (and populated the mapping in _tables). But with set operations specifically, - // we must call RegisterInferredTypeMapping *before* visiting, to infer from one side to the other so that that - // inference can propagate to subqueries nested within the set operation (chicken and egg problem). - // This only results in RegisterInferredTypeMapping being called when it doesn't have it (i.e. _inferredColumns - // contains more than it has to). - - if (projection1.TypeMapping is not null) - { - RegisterInferredTypeMapping(column2, projection1.TypeMapping); - } - - if (projection2.TypeMapping is not null) - { - RegisterInferredTypeMapping(column1, projection2.TypeMapping); - } - - return base.VisitExtension(node); - } - - // Record state on the SelectExpression and ProjectionExpression so that we can associate ColumnExpressions to the - // projections they're in (see below). - case SelectExpression selectExpression: - { - var parentSelectTableAlias = _currentSelectTableAlias; - _currentSelectTableAlias = selectExpression.Alias; - var visited = base.VisitExtension(selectExpression); - _currentSelectTableAlias = parentSelectTableAlias; - return visited; - } - - case ProjectionExpression projectionExpression: - { - var parentProjectionExpression = _currentProjectionExpression; - _currentProjectionExpression = projectionExpression; - var visited = base.VisitExtension(projectionExpression); - _currentProjectionExpression = parentProjectionExpression; - return visited; - } - - // When visiting subqueries, we want to propagate the inferred type mappings from above into the subquery, recursively. - // So we record state above to know which subquery and projection we're visiting; when visiting columns inside a projection - // which has an inferred type mapping from above, we register the inferred type mapping for that column too. - case ColumnExpression { TypeMapping: null } columnExpression - when _currentSelectTableAlias is not null - && _currentProjectionExpression is not null - && _inferredColumns.TryGetValue( - (_currentSelectTableAlias, _currentProjectionExpression.Alias), out var inferredTypeMapping) - && inferredTypeMapping is not null - && WasMaybeOriginallyUntyped(columnExpression): - { - RegisterInferredTypeMapping(columnExpression, inferredTypeMapping); - return base.VisitExtension(node); - } - - case ShapedQueryExpression shapedQueryExpression: - return shapedQueryExpression.UpdateQueryExpression(Visit(shapedQueryExpression.QueryExpression)); - - default: - return base.VisitExtension(node); - } - - bool WasMaybeOriginallyUntyped(ColumnExpression columnExpression) - { - var found = _tableAliasMap.TryGetValue(columnExpression.TableAlias, out var table); - Check.DebugAssert(found, $"Column '{columnExpression}' points to a table that isn't in scope"); - - return table switch - { - // TableExpressions are always fully-typed, with type mappings coming from the model - TableExpression - => false, - - // FromSqlExpressions always receive the default type mapping for the projected element type - we never need to infer - // them. - FromSqlExpression - => false, - - SelectExpression subquery - => subquery.Projection.FirstOrDefault(p => p.Alias == columnExpression.Name) is { Expression.TypeMapping: null }, - - JoinExpressionBase - => throw new UnreachableException("Impossible: nested join"), - - // Any other table expression is considered a root (TableValuedFunctionExpression, ValuesExpression...) which *may* be - // untyped, so we record the possible inference (note that TableValuedFunctionExpression may be typed, or not) - _ => true, - }; - } - - SqlExpression UnwrapConvert(SqlExpression expression) - => expression is SqlUnaryExpression { OperatorType: ExpressionType.Convert } convert - ? UnwrapConvert(convert.Operand) - : expression; - } - - private void RegisterInferredTypeMapping(ColumnExpression columnExpression, RelationalTypeMapping inferredTypeMapping) - { - var tableAlias = columnExpression.TableAlias; - - if (_inferredColumns.TryGetValue((tableAlias, columnExpression.Name), out var knownTypeMapping) - && knownTypeMapping is not null - && inferredTypeMapping.StoreType != knownTypeMapping.StoreType) - { - // A different type mapping was already inferred for this column - we have a conflict. - // Null out the value for the inferred type mapping as an indication of the conflict. If it turns out that we need the - // inferred mapping later, during the application phase, we'll throw an exception at that point (not all the inferred type - // mappings here will actually be needed, so we don't want to needlessly throw here). - _inferredColumns[(tableAlias, columnExpression.Name)] = null; - return; - } - - _inferredColumns[(tableAlias, columnExpression.Name)] = inferredTypeMapping; - } - } - - /// - /// A visitor executed at the end of translation, which verifies that all nodes have a type mapping, - /// and applies type mappings inferred for queryable constants (VALUES) and parameters (e.g. OPENJSON) back on their root tables. - /// - protected class RelationalInferredTypeMappingApplier : ExpressionVisitor - { - private readonly ISqlExpressionFactory _sqlExpressionFactory; - private SelectExpression? _currentSelectExpression; - - /// - /// The inferred type mappings to be applied back on their query roots. - /// - private readonly IReadOnlyDictionary<(string TableAlias, string ColumnName), RelationalTypeMapping?> _inferredTypeMappings; - - /// - /// Creates a new instance of the class. - /// - /// The model. - /// The SQL expression factory. - /// The inferred type mappings to be applied back on their query roots. - public RelationalInferredTypeMappingApplier( - IModel model, - ISqlExpressionFactory sqlExpressionFactory, - IReadOnlyDictionary<(string, string), RelationalTypeMapping?> inferredTypeMappings) - { - Model = model; - _sqlExpressionFactory = sqlExpressionFactory; - _inferredTypeMappings = inferredTypeMappings; - } - - /// - /// The model. - /// - protected virtual IModel Model { get; } - - /// - /// Attempts to find an inferred type mapping for the given table column. - /// - /// The alias of the table containing the column for which to find the inferred type mapping. - /// The name of the column for which to find the inferred type mapping. - /// The inferred type mapping, or if none could be found. - /// Whether an inferred type mapping could be found. - protected virtual bool TryGetInferredTypeMapping( - string tableAlias, - string columnName, - [NotNullWhen(true)] out RelationalTypeMapping? inferredTypeMapping) - { - if (_inferredTypeMappings.TryGetValue((tableAlias, columnName), out inferredTypeMapping)) - { - // The inferred type mapping scanner records a null when two conflicting type mappings were inferred for the same - // column. - if (inferredTypeMapping is null) - { - throw new InvalidOperationException( - RelationalStrings.ConflictingTypeMappingsInferredForColumn(columnName)); - } - - return true; - } - - inferredTypeMapping = null; - return false; - } - - /// - protected override Expression VisitExtension(Expression expression) - { - switch (expression) - { - case ColumnExpression { TypeMapping: null } columnExpression - when TryGetInferredTypeMapping(columnExpression.TableAlias, columnExpression.Name, out var typeMapping): - return columnExpression.ApplyTypeMapping(typeMapping); - - case SelectExpression selectExpression: - var parentSelectExpression = _currentSelectExpression; - _currentSelectExpression = selectExpression; - var visited = base.VisitExtension(expression); - _currentSelectExpression = parentSelectExpression; - return visited; - - // For ValueExpression, apply the inferred type mapping on all constants inside. - case ValuesExpression valuesExpression: - // By default, the ValuesExpression also contains an ordering by a synthetic increasing _ord. If the containing - // SelectExpression doesn't project it out or require it (limit/offset), strip that out. - // TODO: Strictly-speaking, stripping the ordering doesn't belong in this visitor which is about applying type mappings - return ApplyTypeMappingsOnValuesExpression( - valuesExpression, - stripOrdering: _currentSelectExpression is { Limit: null, Offset: null } - && !_currentSelectExpression.Projection.Any( - p => p.Expression is ColumnExpression { Name: ValuesOrderingColumnName } c - && c.TableAlias == valuesExpression.Alias)); - - // SqlExpressions without an inferred type mapping indicates a problem in EF - everything should have been inferred. - // One exception is SqlFragmentExpression, which never has a type mapping. - case SqlExpression { TypeMapping: null } sqlExpression and not SqlFragmentExpression and not ColumnExpression: - throw new InvalidOperationException(RelationalStrings.NullTypeMappingInSqlTree(sqlExpression.Print())); - - case ShapedQueryExpression shapedQueryExpression: - return shapedQueryExpression.UpdateQueryExpression(Visit(shapedQueryExpression.QueryExpression)); - - default: - return base.VisitExtension(expression); - } - } - - /// - /// Applies the given type mappings to the values projected out by the given . - /// As an optimization, it can also strip the first _ord column if it's determined that it isn't needed (most cases). - /// - /// The to apply the mappings to. - /// Whether to strip the _ord column. - protected virtual ValuesExpression ApplyTypeMappingsOnValuesExpression(ValuesExpression valuesExpression, bool stripOrdering) - { - var inferredTypeMappings = TryGetInferredTypeMapping(valuesExpression.Alias, ValuesValueColumnName, out var typeMapping) - ? [null, typeMapping] - : new RelationalTypeMapping?[] { null, null }; - - Check.DebugAssert( - valuesExpression.ColumnNames[0] == ValuesOrderingColumnName, "First ValuesExpression column isn't the ordering column"); - var newColumnNames = stripOrdering - ? valuesExpression.ColumnNames.Skip(1).ToArray() - : valuesExpression.ColumnNames; - - var newRowValues = new RowValueExpression[valuesExpression.RowValues.Count]; - for (var i = 0; i < newRowValues.Length; i++) - { - var rowValue = valuesExpression.RowValues[i]; - var newValues = new SqlExpression[newColumnNames.Count]; - for (var j = 0; j < valuesExpression.ColumnNames.Count; j++) - { - if (j == 0 && stripOrdering) - { - continue; - } - - var value = rowValue.Values[j]; - - var inferredTypeMapping = inferredTypeMappings[j]; - if (inferredTypeMapping is not null && value.TypeMapping is null) - { - 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); - } - } - - newValues[j - (stripOrdering ? 1 : 0)] = value; - } - - newRowValues[i] = new RowValueExpression(newValues); - } - - return new ValuesExpression(valuesExpression.Alias, newRowValues, newColumnNames); - } - } + [Obsolete("Extend RelationalTypeMappingPostprocessor instead")] + protected class RelationalInferredTypeMappingApplier; } diff --git a/src/EFCore.Relational/Query/RelationalTypeMappingPostprocessor.cs b/src/EFCore.Relational/Query/RelationalTypeMappingPostprocessor.cs new file mode 100644 index 00000000000..23f16f894ea --- /dev/null +++ b/src/EFCore.Relational/Query/RelationalTypeMappingPostprocessor.cs @@ -0,0 +1,413 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System.Diagnostics.CodeAnalysis; +using Microsoft.EntityFrameworkCore.Query.SqlExpressions; + +namespace Microsoft.EntityFrameworkCore.Query; + +/// +/// A visitor executed after translation, which verifies that all nodes have a type mapping, +/// and applies type mappings inferred for queryable constants (VALUES) and parameters (e.g. OPENJSON) back on their root tables. +/// +public class RelationalTypeMappingPostprocessor : ExpressionVisitor +{ + private readonly ISqlExpressionFactory _sqlExpressionFactory; + private SelectExpression? _currentSelectExpression; + + /// + /// The inferred type mappings to be applied back on their query roots. + /// + private IReadOnlyDictionary<(string TableAlias, string ColumnName), RelationalTypeMapping?> _inferredTypeMappings = null!; + + /// + /// Creates a new instance of the class. + /// + /// Parameter object containing dependencies for this class. + /// Parameter object containing relational dependencies for this class. + /// The query compilation context object to use. + public RelationalTypeMappingPostprocessor( + QueryTranslationPostprocessorDependencies dependencies, + RelationalQueryTranslationPostprocessorDependencies relationalDependencies, + RelationalQueryCompilationContext queryCompilationContext) + { + Model = queryCompilationContext.Model; + _sqlExpressionFactory = relationalDependencies.SqlExpressionFactory; + } + + /// + /// The model. + /// + protected virtual IModel Model { get; } + + /// + /// Processes type mappings in the expression tree. + /// + /// The expression tree. + public virtual Expression Process(Expression expression) + { + // If any constant/parameter query roots exist in the query, their columns don't yet have a type mapping. + // First, scan the query tree for inferred type mappings (e.g. based on a comparison of those columns to some regular column + // with a type mapping). + _inferredTypeMappings = new ColumnTypeMappingScanner().Scan(expression); + + // Then, apply those type mappings back on the constant/parameter tables (e.g. ValuesExpression). + var visited = Visit(expression); + + return visited; + } + + /// + /// Attempts to find an inferred type mapping for the given table column. + /// + /// The alias of the table containing the column for which to find the inferred type mapping. + /// The name of the column for which to find the inferred type mapping. + /// The inferred type mapping, or if none could be found. + /// Whether an inferred type mapping could be found. + protected virtual bool TryGetInferredTypeMapping( + string tableAlias, + string columnName, + [NotNullWhen(true)] out RelationalTypeMapping? inferredTypeMapping) + { + if (_inferredTypeMappings.TryGetValue((tableAlias, columnName), out inferredTypeMapping)) + { + // The inferred type mapping scanner records a null when two conflicting type mappings were inferred for the same + // column. + if (inferredTypeMapping is null) + { + throw new InvalidOperationException( + RelationalStrings.ConflictingTypeMappingsInferredForColumn(columnName)); + } + + return true; + } + + inferredTypeMapping = null; + return false; + } + + /// + protected override Expression VisitExtension(Expression expression) + { + switch (expression) + { + case ColumnExpression { TypeMapping: null } columnExpression + when TryGetInferredTypeMapping(columnExpression.TableAlias, columnExpression.Name, out var typeMapping): + return columnExpression.ApplyTypeMapping(typeMapping); + + case SelectExpression selectExpression: + var parentSelectExpression = _currentSelectExpression; + _currentSelectExpression = selectExpression; + var visited = base.VisitExtension(expression); + _currentSelectExpression = parentSelectExpression; + return visited; + + // For ValueExpression, apply the inferred type mapping on all constants inside. + case ValuesExpression valuesExpression: + // By default, the ValuesExpression also contains an ordering by a synthetic increasing _ord. If the containing + // SelectExpression doesn't project it out or require it (limit/offset), strip that out. + // TODO: Strictly-speaking, stripping the ordering doesn't belong in this visitor which is about applying type mappings + return ApplyTypeMappingsOnValuesExpression( + valuesExpression, + stripOrdering: _currentSelectExpression is { Limit: null, Offset: null } + && !_currentSelectExpression.Projection.Any( + p => p.Expression is ColumnExpression + { + Name: RelationalQueryableMethodTranslatingExpressionVisitor.ValuesOrderingColumnName + } c + && c.TableAlias == valuesExpression.Alias)); + + // SqlExpressions without an inferred type mapping indicates a problem in EF - everything should have been inferred. + // One exception is SqlFragmentExpression, which never has a type mapping. + case SqlExpression { TypeMapping: null } sqlExpression and not SqlFragmentExpression and not ColumnExpression: + throw new InvalidOperationException(RelationalStrings.NullTypeMappingInSqlTree(sqlExpression.Print())); + + case ShapedQueryExpression shapedQueryExpression: + return shapedQueryExpression.UpdateQueryExpression(Visit(shapedQueryExpression.QueryExpression)); + + default: + return base.VisitExtension(expression); + } + } + + /// + /// Applies the given type mappings to the values projected out by the given . + /// As an optimization, it can also strip the first _ord column if it's determined that it isn't needed (most cases). + /// + /// The to apply the mappings to. + /// Whether to strip the _ord column. + protected virtual ValuesExpression ApplyTypeMappingsOnValuesExpression(ValuesExpression valuesExpression, bool stripOrdering) + { + var inferredTypeMappings = TryGetInferredTypeMapping( + valuesExpression.Alias, RelationalQueryableMethodTranslatingExpressionVisitor.ValuesValueColumnName, out var typeMapping) + ? [null, typeMapping] + : new RelationalTypeMapping?[] { null, null }; + + Check.DebugAssert( + valuesExpression.ColumnNames[0] == RelationalQueryableMethodTranslatingExpressionVisitor.ValuesOrderingColumnName, + "First ValuesExpression column isn't the ordering column"); + var newColumnNames = stripOrdering + ? valuesExpression.ColumnNames.Skip(1).ToArray() + : valuesExpression.ColumnNames; + + var newRowValues = new RowValueExpression[valuesExpression.RowValues.Count]; + for (var i = 0; i < newRowValues.Length; i++) + { + var rowValue = valuesExpression.RowValues[i]; + var newValues = new SqlExpression[newColumnNames.Count]; + for (var j = 0; j < valuesExpression.ColumnNames.Count; j++) + { + if (j == 0 && stripOrdering) + { + continue; + } + + var value = rowValue.Values[j]; + + var inferredTypeMapping = inferredTypeMappings[j]; + if (inferredTypeMapping is not null && value.TypeMapping is null) + { + 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); + } + } + + newValues[j - (stripOrdering ? 1 : 0)] = value; + } + + newRowValues[i] = new RowValueExpression(newValues); + } + + return new ValuesExpression(valuesExpression.Alias, newRowValues, newColumnNames); + } + + /// + /// A visitor which scans an expression tree and attempts to find columns for which we were missing type mappings (projected out + /// of queryable constant/parameter), and those type mappings have been inferred. + /// + /// + /// + /// This handles two cases: (1) an untyped column which type-inferred in the regular way, e.g. through comparison to a typed + /// column, and (2) set operations where on side is typed and the other is untyped. + /// + /// + /// Note that this visitor follows type columns across subquery projections. That is, if a root constant/parameter is buried + /// within subqueries, and somewhere above the column projected out of a subquery is inferred, this is picked up and propagated + /// all the way down. + /// + /// + /// The visitor does not change the query tree in any way - it only populates the inferred type mappings it identified in + /// the given dictionary; actual application of the inferred type mappings happens later in + /// . We can't do this in a single pass since untyped roots + /// (e.g. may get visited before the type-inferred column referring to them (e.g. CROSS APPLY, + /// correlated subquery). + /// + /// + private sealed class ColumnTypeMappingScanner : ExpressionVisitor + { + private readonly Dictionary<(string TableAlias, string ColumnName), RelationalTypeMapping?> _inferredColumns = new(); + + /// + /// A mapping of table aliases to the instances; these are used to check the table type + /// when we encounter a typed column pointing to it, and avoid recording inferred type mappings where we know the table + /// doesn't need to be inferred from the column. + /// + private readonly Dictionary _tableAliasMap = new(); + + private string? _currentSelectTableAlias; + private ProjectionExpression? _currentProjectionExpression; + + public IReadOnlyDictionary<(string, string), RelationalTypeMapping?> Scan(Expression expression) + { + _inferredColumns.Clear(); + _tableAliasMap.Clear(); + + Visit(expression); + + return _inferredColumns; + } + + protected override Expression VisitExtension(Expression node) + { + if (node is TableExpressionBase { Alias: string tableAlias } table) + { + _tableAliasMap[tableAlias] = table.UnwrapJoin(); + } + + switch (node) + { + // A column on a table which was possibly originally untyped (constant/parameter root or a subquery projection of one), + // which now does have a type mapping - this would mean in got inferred in the usual manner (comparison with typed column). + // Registered the inferred type mapping so it can be later applied back to its table, if it's untyped. + case ColumnExpression { TypeMapping: { } typeMapping } c when WasMaybeOriginallyUntyped(c): + { + RegisterInferredTypeMapping(c, typeMapping); + + return base.VisitExtension(node); + } + + // Similar to the above, but with ScalarSubqueryExpression the inferred type mapping is on the expression itself, while the + // ColumnExpression we need is on the subquery's projection. + case ScalarSubqueryExpression + { + TypeMapping: { } typeMapping, + Subquery.Projection: [{ Expression: ColumnExpression columnExpression }] + }: + { + var visitedSubquery = base.VisitExtension(node); + + if (WasMaybeOriginallyUntyped(columnExpression)) + { + RegisterInferredTypeMapping(columnExpression, typeMapping); + } + + return visitedSubquery; + } + + // InExpression over a subquery: apply the item's type mapping on the subquery + case InExpression + { + Item.TypeMapping: { } typeMapping, + Subquery.Projection: [{ Expression: ColumnExpression columnExpression }] + }: + { + var visited = base.VisitExtension(node); + + if (WasMaybeOriginallyUntyped(columnExpression)) + { + RegisterInferredTypeMapping(columnExpression, typeMapping); + } + + return visited; + } + + // For set operations involving a leg with a type mapping (e.g. some column) and a leg without one (queryable constant or + // parameter), we infer the missing type mapping from the other side. + case SetOperationBase + { + Source1.Projection: [{ Expression: var projection1 }], + Source2.Projection: [{ Expression: var projection2 }] + } + when UnwrapConvert(projection1) is ColumnExpression column1 && UnwrapConvert(projection2) is ColumnExpression column2: + { + // Note that we can't use WasMaybeOriginallyUntyped() here like in the other cases, since that only works after we've + // visited the table the column points to (and populated the mapping in _tables). But with set operations specifically, + // we must call RegisterInferredTypeMapping *before* visiting, to infer from one side to the other so that that + // inference can propagate to subqueries nested within the set operation (chicken and egg problem). + // This only results in RegisterInferredTypeMapping being called when it doesn't have it (i.e. _inferredColumns + // contains more than it has to). + + if (projection1.TypeMapping is not null) + { + RegisterInferredTypeMapping(column2, projection1.TypeMapping); + } + + if (projection2.TypeMapping is not null) + { + RegisterInferredTypeMapping(column1, projection2.TypeMapping); + } + + return base.VisitExtension(node); + } + + // Record state on the SelectExpression and ProjectionExpression so that we can associate ColumnExpressions to the + // projections they're in (see below). + case SelectExpression selectExpression: + { + var parentSelectTableAlias = _currentSelectTableAlias; + _currentSelectTableAlias = selectExpression.Alias; + var visited = base.VisitExtension(selectExpression); + _currentSelectTableAlias = parentSelectTableAlias; + return visited; + } + + case ProjectionExpression projectionExpression: + { + var parentProjectionExpression = _currentProjectionExpression; + _currentProjectionExpression = projectionExpression; + var visited = base.VisitExtension(projectionExpression); + _currentProjectionExpression = parentProjectionExpression; + return visited; + } + + // When visiting subqueries, we want to propagate the inferred type mappings from above into the subquery, recursively. + // So we record state above to know which subquery and projection we're visiting; when visiting columns inside a projection + // which has an inferred type mapping from above, we register the inferred type mapping for that column too. + case ColumnExpression { TypeMapping: null } columnExpression + when _currentSelectTableAlias is not null + && _currentProjectionExpression is not null + && _inferredColumns.TryGetValue( + (_currentSelectTableAlias, _currentProjectionExpression.Alias), out var inferredTypeMapping) + && inferredTypeMapping is not null + && WasMaybeOriginallyUntyped(columnExpression): + { + RegisterInferredTypeMapping(columnExpression, inferredTypeMapping); + return base.VisitExtension(node); + } + + case ShapedQueryExpression shapedQueryExpression: + return shapedQueryExpression.UpdateQueryExpression(Visit(shapedQueryExpression.QueryExpression)); + + default: + return base.VisitExtension(node); + } + + bool WasMaybeOriginallyUntyped(ColumnExpression columnExpression) + { + var found = _tableAliasMap.TryGetValue(columnExpression.TableAlias, out var table); + Check.DebugAssert(found, $"Column '{columnExpression}' points to a table that isn't in scope"); + + return table switch + { + // TableExpressions are always fully-typed, with type mappings coming from the model + TableExpression + => false, + + // FromSqlExpressions always receive the default type mapping for the projected element type - we never need to infer + // them. + FromSqlExpression + => false, + + SelectExpression subquery + => subquery.Projection.FirstOrDefault(p => p.Alias == columnExpression.Name) is { Expression.TypeMapping: null }, + + JoinExpressionBase + => throw new UnreachableException("Impossible: nested join"), + + // Any other table expression is considered a root (TableValuedFunctionExpression, ValuesExpression...) which *may* be + // untyped, so we record the possible inference (note that TableValuedFunctionExpression may be typed, or not) + _ => true, + }; + } + + SqlExpression UnwrapConvert(SqlExpression expression) + => expression is SqlUnaryExpression { OperatorType: ExpressionType.Convert } convert + ? UnwrapConvert(convert.Operand) + : expression; + } + + private void RegisterInferredTypeMapping(ColumnExpression columnExpression, RelationalTypeMapping inferredTypeMapping) + { + var tableAlias = columnExpression.TableAlias; + + if (_inferredColumns.TryGetValue((tableAlias, columnExpression.Name), out var knownTypeMapping) + && knownTypeMapping is not null + && inferredTypeMapping.StoreType != knownTypeMapping.StoreType) + { + // A different type mapping was already inferred for this column - we have a conflict. + // Null out the value for the inferred type mapping as an indication of the conflict. If it turns out that we need the + // inferred mapping later, during the application phase, we'll throw an exception at that point (not all the inferred type + // mappings here will actually be needed, so we don't want to needlessly throw here). + _inferredColumns[(tableAlias, columnExpression.Name)] = null; + return; + } + + _inferredColumns[(tableAlias, columnExpression.Name)] = inferredTypeMapping; + } + } +} diff --git a/src/EFCore.SqlServer/Query/Internal/SqlServerQueryTranslationPostprocessor.cs b/src/EFCore.SqlServer/Query/Internal/SqlServerQueryTranslationPostprocessor.cs index 413da4c9ca3..39ac30caf08 100644 --- a/src/EFCore.SqlServer/Query/Internal/SqlServerQueryTranslationPostprocessor.cs +++ b/src/EFCore.SqlServer/Query/Internal/SqlServerQueryTranslationPostprocessor.cs @@ -29,11 +29,11 @@ public class SqlServerQueryTranslationPostprocessor : RelationalQueryTranslation public SqlServerQueryTranslationPostprocessor( QueryTranslationPostprocessorDependencies dependencies, RelationalQueryTranslationPostprocessorDependencies relationalDependencies, - SqlServerQueryCompilationContext queryCompilationContext, - IRelationalTypeMappingSource typeMappingSource) + SqlServerQueryCompilationContext queryCompilationContext) : base(dependencies, relationalDependencies, queryCompilationContext) { - _jsonPostprocessor = new SqlServerJsonPostprocessor(typeMappingSource, relationalDependencies.SqlExpressionFactory); + _jsonPostprocessor = new SqlServerJsonPostprocessor( + relationalDependencies.TypeMappingSource, relationalDependencies.SqlExpressionFactory); } /// @@ -52,6 +52,15 @@ public override Expression Process(Expression query) return query2; } + /// + /// This is an internal API that supports the Entity Framework Core infrastructure and not subject to + /// the same compatibility standards as public APIs. It may be changed or removed without notice in + /// any release. You should only use it directly in your code with extreme caution and knowing that + /// doing so can result in application failures when updating to a new Entity Framework Core release. + /// + protected override Expression ProcessTypeMappings(Expression expression) + => new SqlServerTypeMappingPostprocessor(Dependencies, RelationalDependencies, RelationalQueryCompilationContext).Process(expression); + /// /// This is an internal API that supports the Entity Framework Core infrastructure and not subject to /// the same compatibility standards as public APIs. It may be changed or removed without notice in diff --git a/src/EFCore.SqlServer/Query/Internal/SqlServerQueryTranslationPostprocessorFactory.cs b/src/EFCore.SqlServer/Query/Internal/SqlServerQueryTranslationPostprocessorFactory.cs index e5745ecce75..e912edba794 100644 --- a/src/EFCore.SqlServer/Query/Internal/SqlServerQueryTranslationPostprocessorFactory.cs +++ b/src/EFCore.SqlServer/Query/Internal/SqlServerQueryTranslationPostprocessorFactory.cs @@ -46,6 +46,5 @@ public SqlServerQueryTranslationPostprocessorFactory( /// doing so can result in application failures when updating to a new Entity Framework Core release. /// public virtual QueryTranslationPostprocessor Create(QueryCompilationContext queryCompilationContext) - => new SqlServerQueryTranslationPostprocessor( - Dependencies, RelationalDependencies, (SqlServerQueryCompilationContext)queryCompilationContext, _typeMappingSource); + => new SqlServerQueryTranslationPostprocessor(Dependencies, RelationalDependencies, (SqlServerQueryCompilationContext)queryCompilationContext); } diff --git a/src/EFCore.SqlServer/Query/Internal/SqlServerQueryableMethodTranslatingExpressionVisitor.cs b/src/EFCore.SqlServer/Query/Internal/SqlServerQueryableMethodTranslatingExpressionVisitor.cs index d07b18bda4a..3bedb9a1008 100644 --- a/src/EFCore.SqlServer/Query/Internal/SqlServerQueryableMethodTranslatingExpressionVisitor.cs +++ b/src/EFCore.SqlServer/Query/Internal/SqlServerQueryableMethodTranslatingExpressionVisitor.cs @@ -677,106 +677,4 @@ public TemporalAnnotationApplyingExpressionVisitor(Func - /// This is an internal API that supports the Entity Framework Core infrastructure and not subject to - /// the same compatibility standards as public APIs. It may be changed or removed without notice in - /// any release. You should only use it directly in your code with extreme caution and knowing that - /// doing so can result in application failures when updating to a new Entity Framework Core release. - /// - protected override Expression ApplyInferredTypeMappings( - Expression expression, - IReadOnlyDictionary<(string, string), RelationalTypeMapping?> inferredTypeMappings) - => new SqlServerInferredTypeMappingApplier( - RelationalDependencies.Model, _typeMappingSource, _sqlExpressionFactory, inferredTypeMappings).Visit(expression); - - /// - /// This is an internal API that supports the Entity Framework Core infrastructure and not subject to - /// the same compatibility standards as public APIs. It may be changed or removed without notice in - /// any release. You should only use it directly in your code with extreme caution and knowing that - /// doing so can result in application failures when updating to a new Entity Framework Core release. - /// - protected class SqlServerInferredTypeMappingApplier : RelationalInferredTypeMappingApplier - { - private readonly IRelationalTypeMappingSource _typeMappingSource; - - /// - /// This is an internal API that supports the Entity Framework Core infrastructure and not subject to - /// the same compatibility standards as public APIs. It may be changed or removed without notice in - /// any release. You should only use it directly in your code with extreme caution and knowing that - /// doing so can result in application failures when updating to a new Entity Framework Core release. - /// - public SqlServerInferredTypeMappingApplier( - IModel model, - IRelationalTypeMappingSource typeMappingSource, - ISqlExpressionFactory sqlExpressionFactory, - IReadOnlyDictionary<(string, string), RelationalTypeMapping?> inferredTypeMappings) - : base(model, sqlExpressionFactory, inferredTypeMappings) - { - _typeMappingSource = typeMappingSource; - } - - /// - /// This is an internal API that supports the Entity Framework Core infrastructure and not subject to - /// the same compatibility standards as public APIs. It may be changed or removed without notice in - /// any release. You should only use it directly in your code with extreme caution and knowing that - /// doing so can result in application failures when updating to a new Entity Framework Core release. - /// - protected override Expression VisitExtension(Expression expression) - => expression switch - { - SqlServerOpenJsonExpression openJsonExpression - when TryGetInferredTypeMapping(openJsonExpression.Alias, "value", out var typeMapping) - => ApplyTypeMappingsOnOpenJsonExpression(openJsonExpression, new[] { typeMapping }), - - _ => base.VisitExtension(expression) - }; - - /// - /// This is an internal API that supports the Entity Framework Core infrastructure and not subject to - /// the same compatibility standards as public APIs. It may be changed or removed without notice in - /// any release. You should only use it directly in your code with extreme caution and knowing that - /// doing so can result in application failures when updating to a new Entity Framework Core release. - /// - protected virtual SqlServerOpenJsonExpression ApplyTypeMappingsOnOpenJsonExpression( - SqlServerOpenJsonExpression openJsonExpression, - IReadOnlyList typeMappings) - { - Check.DebugAssert(typeMappings.Count == 1, "typeMappings.Count == 1"); - var elementTypeMapping = typeMappings[0]; - - // Constant queryables are translated to VALUES, no need for JSON. - // Column queryables have their type mapping from the model, so we don't ever need to apply an inferred mapping on them. - if (openJsonExpression.JsonExpression is not SqlParameterExpression { TypeMapping: null } parameterExpression) - { - Check.DebugAssert( - openJsonExpression.JsonExpression.TypeMapping is not null, - "Non-parameter expression without a type mapping in ApplyTypeMappingsOnOpenJsonExpression"); - return openJsonExpression; - } - - Check.DebugAssert( - openJsonExpression.Path is null, "OpenJsonExpression path is non-null when applying an inferred type mapping"); - Check.DebugAssert( - openJsonExpression.ColumnInfos is null, "OpenJsonExpression has no ColumnInfos when applying an inferred type mapping"); - - // We need to apply the inferred type mapping in two places: the collection type mapping on the parameter expanded by OPENJSON, - // and on the WITH clause determining the conversion out on the SQL Server side - - // First, find the collection type mapping and apply it to the parameter - if (_typeMappingSource.FindMapping(parameterExpression.Type, Model, elementTypeMapping) is not SqlServerStringTypeMapping - { - ElementTypeMapping: not null - } - parameterTypeMapping) - { - throw new UnreachableException("A SqlServerStringTypeMapping collection type mapping could not be found"); - } - - return openJsonExpression.Update( - parameterExpression.ApplyTypeMapping(parameterTypeMapping), - path: null, - new[] { new SqlServerOpenJsonExpression.ColumnInfo("value", elementTypeMapping, []) }); - } - } } diff --git a/src/EFCore.SqlServer/Query/Internal/SqlServerTypeMappingPostprocessor.cs b/src/EFCore.SqlServer/Query/Internal/SqlServerTypeMappingPostprocessor.cs new file mode 100644 index 00000000000..aa13da2fbc6 --- /dev/null +++ b/src/EFCore.SqlServer/Query/Internal/SqlServerTypeMappingPostprocessor.cs @@ -0,0 +1,94 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using Microsoft.EntityFrameworkCore.Query.SqlExpressions; +using Microsoft.EntityFrameworkCore.SqlServer.Storage.Internal; + +namespace Microsoft.EntityFrameworkCore.SqlServer.Query.Internal; + +/// +/// This is an internal API that supports the Entity Framework Core infrastructure and not subject to +/// the same compatibility standards as public APIs. It may be changed or removed without notice in +/// any release. You should only use it directly in your code with extreme caution and knowing that +/// doing so can result in application failures when updating to a new Entity Framework Core release. +/// +public class SqlServerTypeMappingPostprocessor : RelationalTypeMappingPostprocessor +{ + private readonly IRelationalTypeMappingSource _typeMappingSource; + + /// + /// This is an internal API that supports the Entity Framework Core infrastructure and not subject to + /// the same compatibility standards as public APIs. It may be changed or removed without notice in + /// any release. You should only use it directly in your code with extreme caution and knowing that + /// doing so can result in application failures when updating to a new Entity Framework Core release. + /// + public SqlServerTypeMappingPostprocessor( + QueryTranslationPostprocessorDependencies dependencies, + RelationalQueryTranslationPostprocessorDependencies relationalDependencies, + RelationalQueryCompilationContext queryCompilationContext) + : base(dependencies, relationalDependencies, queryCompilationContext) + => _typeMappingSource = relationalDependencies.TypeMappingSource; + + /// + /// This is an internal API that supports the Entity Framework Core infrastructure and not subject to + /// the same compatibility standards as public APIs. It may be changed or removed without notice in + /// any release. You should only use it directly in your code with extreme caution and knowing that + /// doing so can result in application failures when updating to a new Entity Framework Core release. + /// + protected override Expression VisitExtension(Expression expression) + => expression switch + { + SqlServerOpenJsonExpression openJsonExpression + when TryGetInferredTypeMapping(openJsonExpression.Alias, "value", out var typeMapping) + => ApplyTypeMappingsOnOpenJsonExpression(openJsonExpression, new[] { typeMapping }), + + _ => base.VisitExtension(expression) + }; + + /// + /// This is an internal API that supports the Entity Framework Core infrastructure and not subject to + /// the same compatibility standards as public APIs. It may be changed or removed without notice in + /// any release. You should only use it directly in your code with extreme caution and knowing that + /// doing so can result in application failures when updating to a new Entity Framework Core release. + /// + protected virtual SqlServerOpenJsonExpression ApplyTypeMappingsOnOpenJsonExpression( + SqlServerOpenJsonExpression openJsonExpression, + IReadOnlyList typeMappings) + { + Check.DebugAssert(typeMappings.Count == 1, "typeMappings.Count == 1"); + var elementTypeMapping = typeMappings[0]; + + // Constant queryables are translated to VALUES, no need for JSON. + // Column queryables have their type mapping from the model, so we don't ever need to apply an inferred mapping on them. + if (openJsonExpression.JsonExpression is not SqlParameterExpression { TypeMapping: null } parameterExpression) + { + Check.DebugAssert( + openJsonExpression.JsonExpression.TypeMapping is not null, + "Non-parameter expression without a type mapping in ApplyTypeMappingsOnOpenJsonExpression"); + return openJsonExpression; + } + + Check.DebugAssert( + openJsonExpression.Path is null, "OpenJsonExpression path is non-null when applying an inferred type mapping"); + Check.DebugAssert( + openJsonExpression.ColumnInfos is null, "OpenJsonExpression has no ColumnInfos when applying an inferred type mapping"); + + // We need to apply the inferred type mapping in two places: the collection type mapping on the parameter expanded by OPENJSON, + // and on the WITH clause determining the conversion out on the SQL Server side + + // First, find the collection type mapping and apply it to the parameter + if (_typeMappingSource.FindMapping(parameterExpression.Type, Model, elementTypeMapping) is not SqlServerStringTypeMapping + { + ElementTypeMapping: not null + } + parameterTypeMapping) + { + throw new UnreachableException("A SqlServerStringTypeMapping collection type mapping could not be found"); + } + + return openJsonExpression.Update( + parameterExpression.ApplyTypeMapping(parameterTypeMapping), + path: null, + new[] { new SqlServerOpenJsonExpression.ColumnInfo("value", elementTypeMapping, []) }); + } +} diff --git a/src/EFCore.Sqlite.Core/Query/Internal/SqliteQueryTranslationPostprocessor.cs b/src/EFCore.Sqlite.Core/Query/Internal/SqliteQueryTranslationPostprocessor.cs index fbe2003c77b..da19cd424eb 100644 --- a/src/EFCore.Sqlite.Core/Query/Internal/SqliteQueryTranslationPostprocessor.cs +++ b/src/EFCore.Sqlite.Core/Query/Internal/SqliteQueryTranslationPostprocessor.cs @@ -44,6 +44,15 @@ public override Expression Process(Expression query) return result; } + /// + /// This is an internal API that supports the Entity Framework Core infrastructure and not subject to + /// the same compatibility standards as public APIs. It may be changed or removed without notice in + /// any release. You should only use it directly in your code with extreme caution and knowing that + /// doing so can result in application failures when updating to a new Entity Framework Core release. + /// + protected override Expression ProcessTypeMappings(Expression expression) + => new SqliteTypeMappingPostprocessor(Dependencies, RelationalDependencies, RelationalQueryCompilationContext).Process(expression); + private sealed class ApplyValidatingVisitor : ExpressionVisitor { protected override Expression VisitExtension(Expression extensionExpression) diff --git a/src/EFCore.Sqlite.Core/Query/Internal/SqliteQueryableMethodTranslatingExpressionVisitor.cs b/src/EFCore.Sqlite.Core/Query/Internal/SqliteQueryableMethodTranslatingExpressionVisitor.cs index 19f21e3c73a..ad10f9b900c 100644 --- a/src/EFCore.Sqlite.Core/Query/Internal/SqliteQueryableMethodTranslatingExpressionVisitor.cs +++ b/src/EFCore.Sqlite.Core/Query/Internal/SqliteQueryableMethodTranslatingExpressionVisitor.cs @@ -534,141 +534,17 @@ private static Type GetProviderType(SqlExpression expression) ?? expression.Type; /// - /// This is an internal API that supports the Entity Framework Core infrastructure and not subject to - /// the same compatibility standards as public APIs. It may be changed or removed without notice in - /// any release. You should only use it directly in your code with extreme caution and knowing that - /// doing so can result in application failures when updating to a new Entity Framework Core release. + /// Wraps the given expression with any SQL logic necessary to convert a value coming out of a JSON document into the relational value + /// represented by the given type mapping. /// - protected override Expression ApplyInferredTypeMappings( - Expression expression, - IReadOnlyDictionary<(string, string), RelationalTypeMapping?> inferredTypeMappings) - => new SqliteInferredTypeMappingApplier( - RelationalDependencies.Model, _typeMappingSource, _sqlExpressionFactory, inferredTypeMappings).Visit(expression); - - /// + /// /// This is an internal API that supports the Entity Framework Core infrastructure and not subject to /// the same compatibility standards as public APIs. It may be changed or removed without notice in /// any release. You should only use it directly in your code with extreme caution and knowing that /// doing so can result in application failures when updating to a new Entity Framework Core release. - /// - protected class SqliteInferredTypeMappingApplier : RelationalInferredTypeMappingApplier - { - private readonly IRelationalTypeMappingSource _typeMappingSource; - private readonly SqliteSqlExpressionFactory _sqlExpressionFactory; - private Dictionary? _currentSelectInferredTypeMappings; - - /// - /// This is an internal API that supports the Entity Framework Core infrastructure and not subject to - /// the same compatibility standards as public APIs. It may be changed or removed without notice in - /// any release. You should only use it directly in your code with extreme caution and knowing that - /// doing so can result in application failures when updating to a new Entity Framework Core release. - /// - public SqliteInferredTypeMappingApplier( - IModel model, - IRelationalTypeMappingSource typeMappingSource, - SqliteSqlExpressionFactory sqlExpressionFactory, - IReadOnlyDictionary<(string, string), RelationalTypeMapping?> inferredTypeMappings) - : base(model, sqlExpressionFactory, inferredTypeMappings) - { - (_typeMappingSource, _sqlExpressionFactory) = (typeMappingSource, sqlExpressionFactory); - } - - /// - /// This is an internal API that supports the Entity Framework Core infrastructure and not subject to - /// the same compatibility standards as public APIs. It may be changed or removed without notice in - /// any release. You should only use it directly in your code with extreme caution and knowing that - /// doing so can result in application failures when updating to a new Entity Framework Core release. - /// - protected override Expression VisitExtension(Expression expression) - { - switch (expression) - { - case JsonEachExpression jsonEachExpression - when TryGetInferredTypeMapping(jsonEachExpression.Alias, "value", out var typeMapping): - return ApplyTypeMappingsOnJsonEachExpression(jsonEachExpression, typeMapping); - - // Above, we applied the type mapping to the parameter that json_each accepts as an argument. - // But the inferred type mapping also needs to be applied as a SQL conversion on the column projections coming out of the - // SelectExpression containing the json_each call. So we set state to know about json_each tables and their type mappings - // in the immediate SelectExpression, and continue visiting down (see ColumnExpression visitation below). - case SelectExpression selectExpression: - { - Dictionary? previousSelectInferredTypeMappings = null; - - foreach (var table in selectExpression.Tables) - { - if (table is TableValuedFunctionExpression { Name: "json_each", Schema: null, IsBuiltIn: true } jsonEachExpression - && TryGetInferredTypeMapping(jsonEachExpression.Alias, "value", out var inferredTypeMapping)) - { - if (previousSelectInferredTypeMappings is null) - { - previousSelectInferredTypeMappings = _currentSelectInferredTypeMappings; - _currentSelectInferredTypeMappings = new Dictionary(); - } - - _currentSelectInferredTypeMappings![jsonEachExpression.Alias] = inferredTypeMapping; - } - } - - var visited = base.VisitExtension(expression); - - _currentSelectInferredTypeMappings = previousSelectInferredTypeMappings; - - return visited; - } - - // Note that we match also ColumnExpressions which already have a type mapping, i.e. coming out of column collections (as - // opposed to parameter collections, where the type mapping needs to be inferred). This is in order to apply SQL conversion - // logic later in the process, see note in TranslateCollection. - case ColumnExpression { Name: "value" } columnExpression - when _currentSelectInferredTypeMappings?.TryGetValue(columnExpression.TableAlias, out var inferredTypeMapping) is true: - return ApplyJsonSqlConversion( - columnExpression.ApplyTypeMapping(inferredTypeMapping), - _sqlExpressionFactory, - inferredTypeMapping, - columnExpression.IsNullable); - - default: - return base.VisitExtension(expression); - } - } - - /// - /// This is an internal API that supports the Entity Framework Core infrastructure and not subject to - /// the same compatibility standards as public APIs. It may be changed or removed without notice in - /// any release. You should only use it directly in your code with extreme caution and knowing that - /// doing so can result in application failures when updating to a new Entity Framework Core release. - /// - protected virtual JsonEachExpression ApplyTypeMappingsOnJsonEachExpression( - JsonEachExpression jsonEachExpression, - RelationalTypeMapping inferredTypeMapping) - { - // Constant queryables are translated to VALUES, no need for JSON. - // Column queryables have their type mapping from the model, so we don't ever need to apply an inferred mapping on them. - if (jsonEachExpression.Arguments[0] is not SqlParameterExpression parameterExpression) - { - return jsonEachExpression; - } - - if (_typeMappingSource.FindMapping(parameterExpression.Type, Model, inferredTypeMapping) is not SqliteStringTypeMapping - parameterTypeMapping) - { - throw new InvalidOperationException("Type mapping for 'string' could not be found or was not a SqliteStringTypeMapping"); - } - - Check.DebugAssert(parameterTypeMapping.ElementTypeMapping != null, "Collection type mapping missing element mapping."); - - return jsonEachExpression.Update( - parameterExpression.ApplyTypeMapping(parameterTypeMapping), - jsonEachExpression.Path); - } - } - - /// - /// Wraps the given expression with any SQL logic necessary to convert a value coming out of a JSON document into the relational value - /// represented by the given type mapping. - /// - private static SqlExpression ApplyJsonSqlConversion( + /// + [EntityFrameworkInternal] + public static SqlExpression ApplyJsonSqlConversion( SqlExpression expression, SqliteSqlExpressionFactory sqlExpressionFactory, RelationalTypeMapping typeMapping, diff --git a/src/EFCore.Sqlite.Core/Query/Internal/SqliteTypeMappingPostprocessor.cs b/src/EFCore.Sqlite.Core/Query/Internal/SqliteTypeMappingPostprocessor.cs new file mode 100644 index 00000000000..52fe475965b --- /dev/null +++ b/src/EFCore.Sqlite.Core/Query/Internal/SqliteTypeMappingPostprocessor.cs @@ -0,0 +1,127 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using Microsoft.EntityFrameworkCore.Query.SqlExpressions; +using Microsoft.EntityFrameworkCore.Sqlite.Query.SqlExpressions.Internal; +using Microsoft.EntityFrameworkCore.Sqlite.Storage.Internal; + +namespace Microsoft.EntityFrameworkCore.Sqlite.Query.Internal; + +/// +/// This is an internal API that supports the Entity Framework Core infrastructure and not subject to +/// the same compatibility standards as public APIs. It may be changed or removed without notice in +/// any release. You should only use it directly in your code with extreme caution and knowing that +/// doing so can result in application failures when updating to a new Entity Framework Core release. +/// +public class SqliteTypeMappingPostprocessor : RelationalTypeMappingPostprocessor +{ + private readonly IRelationalTypeMappingSource _typeMappingSource; + private readonly SqliteSqlExpressionFactory _sqlExpressionFactory; + private Dictionary? _currentSelectInferredTypeMappings; + + /// + /// This is an internal API that supports the Entity Framework Core infrastructure and not subject to + /// the same compatibility standards as public APIs. It may be changed or removed without notice in + /// any release. You should only use it directly in your code with extreme caution and knowing that + /// doing so can result in application failures when updating to a new Entity Framework Core release. + /// + public SqliteTypeMappingPostprocessor( + QueryTranslationPostprocessorDependencies dependencies, + RelationalQueryTranslationPostprocessorDependencies relationalDependencies, + RelationalQueryCompilationContext queryCompilationContext) + : base(dependencies, relationalDependencies, queryCompilationContext) + { + _typeMappingSource = relationalDependencies.TypeMappingSource; + _sqlExpressionFactory = (SqliteSqlExpressionFactory)relationalDependencies.SqlExpressionFactory; + } + + /// + /// This is an internal API that supports the Entity Framework Core infrastructure and not subject to + /// the same compatibility standards as public APIs. It may be changed or removed without notice in + /// any release. You should only use it directly in your code with extreme caution and knowing that + /// doing so can result in application failures when updating to a new Entity Framework Core release. + /// + protected override Expression VisitExtension(Expression expression) + { + switch (expression) + { + case JsonEachExpression jsonEachExpression + when TryGetInferredTypeMapping(jsonEachExpression.Alias, "value", out var typeMapping): + return ApplyTypeMappingsOnJsonEachExpression(jsonEachExpression, typeMapping); + + // Above, we applied the type mapping to the parameter that json_each accepts as an argument. + // But the inferred type mapping also needs to be applied as a SQL conversion on the column projections coming out of the + // SelectExpression containing the json_each call. So we set state to know about json_each tables and their type mappings + // in the immediate SelectExpression, and continue visiting down (see ColumnExpression visitation below). + case SelectExpression selectExpression: + { + Dictionary? previousSelectInferredTypeMappings = null; + + foreach (var table in selectExpression.Tables) + { + if (table is TableValuedFunctionExpression { Name: "json_each", Schema: null, IsBuiltIn: true } jsonEachExpression + && TryGetInferredTypeMapping(jsonEachExpression.Alias, "value", out var inferredTypeMapping)) + { + if (previousSelectInferredTypeMappings is null) + { + previousSelectInferredTypeMappings = _currentSelectInferredTypeMappings; + _currentSelectInferredTypeMappings = new Dictionary(); + } + + _currentSelectInferredTypeMappings![jsonEachExpression.Alias] = inferredTypeMapping; + } + } + + var visited = base.VisitExtension(expression); + + _currentSelectInferredTypeMappings = previousSelectInferredTypeMappings; + + return visited; + } + + // Note that we match also ColumnExpressions which already have a type mapping, i.e. coming out of column collections (as + // opposed to parameter collections, where the type mapping needs to be inferred). This is in order to apply SQL conversion + // logic later in the process, see note in TranslateCollection. + case ColumnExpression { Name: "value" } columnExpression + when _currentSelectInferredTypeMappings?.TryGetValue(columnExpression.TableAlias, out var inferredTypeMapping) is true: + return SqliteQueryableMethodTranslatingExpressionVisitor.ApplyJsonSqlConversion( + columnExpression.ApplyTypeMapping(inferredTypeMapping), + _sqlExpressionFactory, + inferredTypeMapping, + columnExpression.IsNullable); + + default: + return base.VisitExtension(expression); + } + } + + /// + /// This is an internal API that supports the Entity Framework Core infrastructure and not subject to + /// the same compatibility standards as public APIs. It may be changed or removed without notice in + /// any release. You should only use it directly in your code with extreme caution and knowing that + /// doing so can result in application failures when updating to a new Entity Framework Core release. + /// + protected virtual JsonEachExpression ApplyTypeMappingsOnJsonEachExpression( + JsonEachExpression jsonEachExpression, + RelationalTypeMapping inferredTypeMapping) + { + // Constant queryables are translated to VALUES, no need for JSON. + // Column queryables have their type mapping from the model, so we don't ever need to apply an inferred mapping on them. + if (jsonEachExpression.Arguments[0] is not SqlParameterExpression parameterExpression) + { + return jsonEachExpression; + } + + if (_typeMappingSource.FindMapping(parameterExpression.Type, Model, inferredTypeMapping) is not SqliteStringTypeMapping + parameterTypeMapping) + { + throw new InvalidOperationException("Type mapping for 'string' could not be found or was not a SqliteStringTypeMapping"); + } + + Check.DebugAssert(parameterTypeMapping.ElementTypeMapping != null, "Collection type mapping missing element mapping."); + + return jsonEachExpression.Update( + parameterExpression.ApplyTypeMapping(parameterTypeMapping), + jsonEachExpression.Path); + } +} diff --git a/test/EFCore.Specification.Tests/LazyLoadProxyTestBase.cs b/test/EFCore.Specification.Tests/LazyLoadProxyTestBase.cs index 4b86d69d95a..a29f5fc4606 100644 --- a/test/EFCore.Specification.Tests/LazyLoadProxyTestBase.cs +++ b/test/EFCore.Specification.Tests/LazyLoadProxyTestBase.cs @@ -58,7 +58,7 @@ public virtual void Can_use_proxies_from_multiple_threads_when_navigations_alrea Assert.Equal(children, parent.Children); Assert.Equal(singlePkToPk, parent.SinglePkToPk); Assert.Equal(single, parent.Single); - Assert.Equal(childrenAk, parent.ChildrenAk); + Assert.Equal(childrenAk, parent.ChildrenAk!); Assert.Equal(singleAk, parent.SingleAk); Assert.Equal(childrenShadowFk, parent.ChildrenShadowFk); Assert.Equal(singleShadowFk, parent.SingleShadowFk); From 05c7e4fa5e48a2b217addb0f1cd4f128528458e3 Mon Sep 17 00:00:00 2001 From: Shay Rojansky Date: Thu, 14 Mar 2024 08:24:57 +0100 Subject: [PATCH 2/2] Address review comments --- ...RelationalQueryTranslationPostprocessor.cs | 26 +++++++----- ...yableMethodTranslatingExpressionVisitor.cs | 20 ++------- ...yableMethodTranslatingExpressionVisitor.cs | 41 ++++++++++++++----- .../SqliteTypeMappingPostprocessor.cs | 12 ++++-- 4 files changed, 57 insertions(+), 42 deletions(-) diff --git a/src/EFCore.Relational/Query/RelationalQueryTranslationPostprocessor.cs b/src/EFCore.Relational/Query/RelationalQueryTranslationPostprocessor.cs index 04ea10c8b05..cf650072da0 100644 --- a/src/EFCore.Relational/Query/RelationalQueryTranslationPostprocessor.cs +++ b/src/EFCore.Relational/Query/RelationalQueryTranslationPostprocessor.cs @@ -45,25 +45,29 @@ public RelationalQueryTranslationPostprocessor( /// public override Expression Process(Expression query) { - var query1 = base.Process(query); - var query2 = ProcessTypeMappings(query1); - var query3 = new SelectExpressionProjectionApplyingExpressionVisitor( - ((RelationalQueryCompilationContext)QueryCompilationContext).QuerySplittingBehavior).Visit(query2); - var query4 = Prune(query3); + var afterBase = base.Process(query); + var afterTypeMappings = ProcessTypeMappings(afterBase); + var afterProjectionApplication = new SelectExpressionProjectionApplyingExpressionVisitor( + ((RelationalQueryCompilationContext)QueryCompilationContext).QuerySplittingBehavior) + .Visit(afterTypeMappings); + var afterPruning = Prune(afterProjectionApplication); // TODO: This - and all the verifications below - should happen after all visitors have run, including provider-specific ones. - var query5 = _sqlAliasManager.PostprocessAliases(query4); + var afterAliases = _sqlAliasManager.PostprocessAliases(afterPruning); #if DEBUG // Verifies that all SelectExpression are marked as immutable after this point. - new SelectExpressionMutableVerifyingExpressionVisitor().Visit(query5); + new SelectExpressionMutableVerifyingExpressionVisitor().Visit(afterAliases); #endif - var query6 = new SqlExpressionSimplifyingExpressionVisitor(RelationalDependencies.SqlExpressionFactory, _useRelationalNulls) - .Visit(query5); - var query7 = new RelationalValueConverterCompensatingExpressionVisitor(RelationalDependencies.SqlExpressionFactory).Visit(query6); + var afterSimplification = new SqlExpressionSimplifyingExpressionVisitor( + RelationalDependencies.SqlExpressionFactory, _useRelationalNulls) + .Visit(afterAliases); + var afterValueConverterCompensation = + new RelationalValueConverterCompensatingExpressionVisitor(RelationalDependencies.SqlExpressionFactory) + .Visit(afterSimplification); - return query7; + return afterValueConverterCompensation; } /// diff --git a/src/EFCore.Relational/Query/RelationalQueryableMethodTranslatingExpressionVisitor.cs b/src/EFCore.Relational/Query/RelationalQueryableMethodTranslatingExpressionVisitor.cs index d65226df9b3..7d0076c0598 100644 --- a/src/EFCore.Relational/Query/RelationalQueryableMethodTranslatingExpressionVisitor.cs +++ b/src/EFCore.Relational/Query/RelationalQueryableMethodTranslatingExpressionVisitor.cs @@ -1314,21 +1314,6 @@ protected override ShapedQueryExpression TranslateUnion(ShapedQueryExpression so LambdaExpression lambdaExpression) => TranslateExpression(RemapLambdaBody(shapedQueryExpression, lambdaExpression)); - /// - /// Invoked at the end of top-level translation, applies inferred type mappings for queryable constants/parameters and verifies that - /// all have a type mapping. - /// - /// The query expression to process. - /// - /// Inferred type mappings for queryable constants/parameters collected during translation. These will be applied to the appropriate - /// nodes in the tree. - /// - [Obsolete("Override RelationalQueryTranslationPostprocessor.ProcessTypeMappings() instead.", error: true)] - protected virtual Expression ApplyInferredTypeMappings( - Expression expression, - IReadOnlyDictionary<(string, string), RelationalTypeMapping?> inferredTypeMappings) - => throw new UnreachableException(); - /// /// Determines whether the given is ordered, typically because orderings have been added to it. /// @@ -2097,8 +2082,9 @@ private bool TryExtractBareInlineCollectionValues(ShapedQueryExpression shapedQu } /// - /// This visitor has been obsoleted; extend RelationalTypeMappingPostprocessor instead. + /// This visitor has been obsoleted; Extend RelationalTypeMappingPostprocessor instead, and invoke it from + /// . /// - [Obsolete("Extend RelationalTypeMappingPostprocessor instead")] + [Obsolete("Extend RelationalTypeMappingPostprocessor instead, and invoke it from RelationalQueryTranslationPostprocessor.ProcessTypeMappings().")] protected class RelationalInferredTypeMappingApplier; } diff --git a/src/EFCore.Sqlite.Core/Query/Internal/SqliteQueryableMethodTranslatingExpressionVisitor.cs b/src/EFCore.Sqlite.Core/Query/Internal/SqliteQueryableMethodTranslatingExpressionVisitor.cs index ad10f9b900c..dc9d782e20d 100644 --- a/src/EFCore.Sqlite.Core/Query/Internal/SqliteQueryableMethodTranslatingExpressionVisitor.cs +++ b/src/EFCore.Sqlite.Core/Query/Internal/SqliteQueryableMethodTranslatingExpressionVisitor.cs @@ -23,6 +23,24 @@ public class SqliteQueryableMethodTranslatingExpressionVisitor : RelationalQuery private readonly SqlAliasManager _sqlAliasManager; private readonly bool _areJsonFunctionsSupported; + /// + /// This is an internal API that supports the Entity Framework Core infrastructure and not subject to + /// the same compatibility standards as public APIs. It may be changed or removed without notice in + /// any release. You should only use it directly in your code with extreme caution and knowing that + /// doing so can result in application failures when updating to a new Entity Framework Core release. + /// + [EntityFrameworkInternal] // https://www.sqlite.org/json1.html#jeach + public const string JsonEachKeyColumnName = "key"; + + /// + /// This is an internal API that supports the Entity Framework Core infrastructure and not subject to + /// the same compatibility standards as public APIs. It may be changed or removed without notice in + /// any release. You should only use it directly in your code with extreme caution and knowing that + /// doing so can result in application failures when updating to a new Entity Framework Core release. + /// + [EntityFrameworkInternal] // https://www.sqlite.org/json1.html#jeach + public const string JsonEachValueColumnName = "value"; + /// /// This is an internal API that supports the Entity Framework Core infrastructure and not subject to /// the same compatibility standards as public APIs. It may be changed or removed without notice in @@ -239,12 +257,12 @@ protected override QueryableMethodTranslatingExpressionVisitor CreateSubqueryVis var selectExpression = new SelectExpression( [jsonEachExpression], new ColumnExpression( - "value", + JsonEachValueColumnName, tableAlias, elementClrType.UnwrapNullableType(), elementTypeMapping, isElementNullable ?? elementClrType.IsNullableType()), - identifier: [(new ColumnExpression("key", tableAlias, typeof(int), keyColumnTypeMapping, nullable: false), keyColumnTypeMapping.Comparer)], + identifier: [(new ColumnExpression(JsonEachKeyColumnName, tableAlias, typeof(int), keyColumnTypeMapping, nullable: false), keyColumnTypeMapping.Comparer)], _sqlAliasManager); #pragma warning restore EF1001 // Internal EF Core API usage. @@ -263,7 +281,7 @@ protected override QueryableMethodTranslatingExpressionVisitor CreateSubqueryVis new OrderingExpression( selectExpression.CreateColumnExpression( jsonEachExpression, - "key", + JsonEachKeyColumnName, typeof(int), typeMapping: _typeMappingSource.FindMapping(typeof(int)), columnNullable: false), @@ -325,7 +343,7 @@ protected override ShapedQueryExpression TransformJsonQueryToTable(JsonQueryExpr var selectExpression = CreateSelect( jsonQueryExpression, jsonEachExpression, - "key", + JsonEachKeyColumnName, typeof(int), _typeMappingSource.FindMapping(typeof(int))!); #pragma warning restore EF1001 // Internal EF Core API usage. @@ -334,7 +352,7 @@ protected override ShapedQueryExpression TransformJsonQueryToTable(JsonQueryExpr new OrderingExpression( selectExpression.CreateColumnExpression( jsonEachExpression, - "key", + JsonEachKeyColumnName, typeof(int), typeMapping: _typeMappingSource.FindMapping(typeof(int)), columnNullable: false), @@ -343,7 +361,7 @@ protected override ShapedQueryExpression TransformJsonQueryToTable(JsonQueryExpr var propertyJsonScalarExpression = new Dictionary(); var jsonColumn = selectExpression.CreateColumnExpression( - jsonEachExpression, "value", typeof(string), _typeMappingSource.FindMapping(typeof(string))); // TODO: nullable? + jsonEachExpression, JsonEachValueColumnName, typeof(string), _typeMappingSource.FindMapping(typeof(string))); // TODO: nullable? var containerColumnName = entityType.GetContainerColumnName(); Check.DebugAssert(containerColumnName is not null, "JsonQueryExpression to entity type without a container column name"); @@ -403,7 +421,7 @@ protected override ShapedQueryExpression TransformJsonQueryToTable(JsonQueryExpr var newOuterSelectExpression = CreateSelect( jsonQueryExpression, subquery, - "key", + JsonEachKeyColumnName, typeof(int), _typeMappingSource.FindMapping(typeof(int))!); #pragma warning restore EF1001 // Internal EF Core API usage. @@ -412,7 +430,7 @@ protected override ShapedQueryExpression TransformJsonQueryToTable(JsonQueryExpr new OrderingExpression( selectExpression.CreateColumnExpression( subquery, - "key", + JsonEachKeyColumnName, typeof(int), typeMapping: _typeMappingSource.FindMapping(typeof(int)), columnNullable: false), @@ -453,7 +471,7 @@ protected override ShapedQueryExpression TransformJsonQueryToTable(JsonQueryExpr GroupBy: [], Having: null, IsDistinct: false, - Orderings: [{ Expression: ColumnExpression { Name: "key" } orderingColumn, IsAscending: true }], + Orderings: [{ Expression: ColumnExpression { Name: JsonEachKeyColumnName } orderingColumn, IsAscending: true }], Limit: null, Offset: null } selectExpression @@ -512,7 +530,7 @@ protected override bool IsNaturallyOrdered(SelectExpression selectExpression) Orderings: [ { - Expression: ColumnExpression { Name: "key" } orderingColumn, + Expression: ColumnExpression { Name: JsonEachKeyColumnName } orderingColumn, IsAscending: true } ] @@ -524,7 +542,8 @@ bool IsJsonEachKeyColumn(SelectExpression selectExpression, ColumnExpression ord => selectExpression.Tables.FirstOrDefault(t => t.Alias == orderingColumn.TableAlias)?.UnwrapJoin() is TableExpressionBase table && (table is JsonEachExpression || (table is SelectExpression subquery - && subquery.Projection.FirstOrDefault(p => p.Alias == "key")?.Expression is ColumnExpression projectedColumn + && subquery.Projection.FirstOrDefault(p => p.Alias == JsonEachKeyColumnName)?.Expression is ColumnExpression + projectedColumn && IsJsonEachKeyColumn(subquery, projectedColumn))); } diff --git a/src/EFCore.Sqlite.Core/Query/Internal/SqliteTypeMappingPostprocessor.cs b/src/EFCore.Sqlite.Core/Query/Internal/SqliteTypeMappingPostprocessor.cs index 52fe475965b..6c67cec070a 100644 --- a/src/EFCore.Sqlite.Core/Query/Internal/SqliteTypeMappingPostprocessor.cs +++ b/src/EFCore.Sqlite.Core/Query/Internal/SqliteTypeMappingPostprocessor.cs @@ -46,7 +46,10 @@ protected override Expression VisitExtension(Expression expression) switch (expression) { case JsonEachExpression jsonEachExpression - when TryGetInferredTypeMapping(jsonEachExpression.Alias, "value", out var typeMapping): + when TryGetInferredTypeMapping( + jsonEachExpression.Alias, + SqliteQueryableMethodTranslatingExpressionVisitor.JsonEachValueColumnName, + out var typeMapping): return ApplyTypeMappingsOnJsonEachExpression(jsonEachExpression, typeMapping); // Above, we applied the type mapping to the parameter that json_each accepts as an argument. @@ -60,7 +63,10 @@ when TryGetInferredTypeMapping(jsonEachExpression.Alias, "value", out var typeMa foreach (var table in selectExpression.Tables) { if (table is TableValuedFunctionExpression { Name: "json_each", Schema: null, IsBuiltIn: true } jsonEachExpression - && TryGetInferredTypeMapping(jsonEachExpression.Alias, "value", out var inferredTypeMapping)) + && TryGetInferredTypeMapping( + jsonEachExpression.Alias, + SqliteQueryableMethodTranslatingExpressionVisitor.JsonEachValueColumnName, + out var inferredTypeMapping)) { if (previousSelectInferredTypeMappings is null) { @@ -82,7 +88,7 @@ when TryGetInferredTypeMapping(jsonEachExpression.Alias, "value", out var typeMa // Note that we match also ColumnExpressions which already have a type mapping, i.e. coming out of column collections (as // opposed to parameter collections, where the type mapping needs to be inferred). This is in order to apply SQL conversion // logic later in the process, see note in TranslateCollection. - case ColumnExpression { Name: "value" } columnExpression + case ColumnExpression { Name: SqliteQueryableMethodTranslatingExpressionVisitor.JsonEachValueColumnName } columnExpression when _currentSelectInferredTypeMappings?.TryGetValue(columnExpression.TableAlias, out var inferredTypeMapping) is true: return SqliteQueryableMethodTranslatingExpressionVisitor.ApplyJsonSqlConversion( columnExpression.ApplyTypeMapping(inferredTypeMapping),