Redo SQL table alias management (#32785)

Closes #32784
Fixes #32757
Part of #32778
Fixes #26104

EFCore.sln.DotSettings

@@ -334,6 +334,7 @@ The .NET Foundation licenses this file to you under the MIT license.
 	<s:Boolean x:Key="/Default/UserDictionary/Words/=sqlite/@EntryIndexedValue">True</s:Boolean>
 	<s:Boolean x:Key="/Default/UserDictionary/Words/=subqueries/@EntryIndexedValue">True</s:Boolean>
 	<s:Boolean x:Key="/Default/UserDictionary/Words/=subquery/@EntryIndexedValue">True</s:Boolean>
+	<s:Boolean x:Key="/Default/UserDictionary/Words/=subquery_0027s/@EntryIndexedValue">True</s:Boolean>
 	<s:Boolean x:Key="/Default/UserDictionary/Words/=transactionality/@EntryIndexedValue">True</s:Boolean>
 	<s:Boolean x:Key="/Default/UserDictionary/Words/=unconfigured/@EntryIndexedValue">True</s:Boolean>
 	<s:Boolean x:Key="/Default/UserDictionary/Words/=unignore/@EntryIndexedValue">True</s:Boolean>

src/EFCore.Relational/Infrastructure/EntityFrameworkRelationalServicesBuilder.cs

@@ -63,6 +63,7 @@ public static readonly IDictionary<Type, ServiceCharacteristics> RelationalServi
             { typeof(IRawSqlCommandBuilder), new ServiceCharacteristics(ServiceLifetime.Singleton) },
             { typeof(IQuerySqlGeneratorFactory), new ServiceCharacteristics(ServiceLifetime.Singleton) },
             { typeof(IModificationCommandFactory), new ServiceCharacteristics(ServiceLifetime.Singleton) },
+            { typeof(ISqlAliasManagerFactory), new ServiceCharacteristics(ServiceLifetime.Singleton) },
             { typeof(ICommandBatchPreparer), new ServiceCharacteristics(ServiceLifetime.Scoped) },
             { typeof(IModificationCommandBatchFactory), new ServiceCharacteristics(ServiceLifetime.Scoped) },
             { typeof(IRelationalSqlTranslatingExpressionVisitorFactory), new ServiceCharacteristics(ServiceLifetime.Scoped) },
@@ -187,6 +188,7 @@ public override EntityFrameworkServicesBuilder TryAddCoreServices()
         TryAdd<IRelationalQueryStringFactory, RelationalQueryStringFactory>();
         TryAdd<IQueryCompilationContextFactory, RelationalQueryCompilationContextFactory>();
         TryAdd<IAdHocMapper, RelationalAdHocMapper>();
+        TryAdd<ISqlAliasManagerFactory, SqlAliasManagerFactory>();
 
         ServiceCollectionMap.GetInfrastructure()
             .AddDependencySingleton<RelationalSqlGenerationHelperDependencies>()

src/EFCore.Relational/Query/ISqlAliasManagerFactory.cs

@@ -0,0 +1,15 @@
+// Licensed to the .NET Foundation under one or more agreements.
+// The .NET Foundation licenses this file to you under the MIT license.
+
+namespace Microsoft.EntityFrameworkCore.Query;
+
+/// <summary>
+///     A factory creating managers for SQL aliases, capable of generate uniquified table aliases.
+/// </summary>
+public interface ISqlAliasManagerFactory
+{
+    /// <summary>
+    ///     Creates a new <see cref="SqlAliasManager" />.
+    /// </summary>
+    SqlAliasManager Create();
+}

src/EFCore.Relational/Query/ISqlExpressionFactory.cs

@@ -460,29 +460,31 @@ SqlFunctionExpression NiladicFunction(
     SqlFragmentExpression Fragment(string sql);
 
     /// <summary>
-    ///     Creates a new <see cref="SelectExpression" /> which represents a SELECT in a SQL tree projecting a <see cref="SqlExpression" />
-    ///     or 1 from no table and without any composition.
+    ///     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.
     /// </summary>
-    /// <param name="projection">A <see cref="SqlExpression" /> to project.</param>
-    /// <returns>An expression representing a SELECT in a SQL tree.</returns>
-    SelectExpression Select(SqlExpression? projection);
+    [EntityFrameworkInternal]
+    SelectExpression Select(SqlExpression? projection, SqlAliasManager sqlAliasManager);
 
     /// <summary>
-    ///     Creates a new <see cref="SelectExpression" /> which represents a SELECT in a SQL tree projecting an entity type from
-    ///     a table source created using default mapping in the model.
+    ///     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.
     /// </summary>
-    /// <param name="entityType">An entity type to project.</param>
-    /// <returns>An expression representing a SELECT in a SQL tree.</returns>
-    SelectExpression Select(IEntityType entityType);
+    [EntityFrameworkInternal]
+    SelectExpression Select(IEntityType entityType, SqlAliasManager sqlAliasManager);
 
     /// <summary>
-    ///     Creates a new <see cref="SelectExpression" /> which represents a SELECT in a SQL tree projecting an entity type from
-    ///     a table source.
+    ///     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.
     /// </summary>
-    /// <param name="entityType">An entity type to project.</param>
-    /// <param name="tableExpressionBase">A table source to project from.</param>
-    /// <returns>An expression representing a SELECT in a SQL tree.</returns>
-    SelectExpression Select(IEntityType entityType, TableExpressionBase tableExpressionBase);
+    [EntityFrameworkInternal]
+    SelectExpression Select(IEntityType entityType, TableExpressionBase tableExpressionBase, SqlAliasManager sqlAliasManager);
 
     /// <summary>
     ///     Attempts to creates a new expression that returns the smallest value from a list of expressions, e.g. an invocation of the

src/EFCore.Relational/Query/Internal/RelationalQueryTranslationPostprocessorFactory.cs

@@ -45,5 +45,5 @@ public virtual QueryTranslationPostprocessor Create(QueryCompilationContext quer
         => new RelationalQueryTranslationPostprocessor(
             Dependencies,
             RelationalDependencies,
-            queryCompilationContext);
+            (RelationalQueryCompilationContext)queryCompilationContext);
 }

src/EFCore.Relational/Query/Internal/RelationalQueryableMethodTranslatingExpressionVisitorFactory.cs

@@ -45,5 +45,5 @@ public virtual QueryableMethodTranslatingExpressionVisitor Create(QueryCompilati
         => new RelationalQueryableMethodTranslatingExpressionVisitor(
             Dependencies,
             RelationalDependencies,
-            queryCompilationContext);
+            (RelationalQueryCompilationContext)queryCompilationContext);
 }

src/EFCore.Relational/Query/Internal/SqlAliasManagerFactory.cs

@@ -0,0 +1,22 @@
+// Licensed to the .NET Foundation under one or more agreements.
+// The .NET Foundation licenses this file to you under the MIT license.
+
+namespace Microsoft.EntityFrameworkCore.Query.Internal;
+
+/// <summary>
+///     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.
+/// </summary>
+public class SqlAliasManagerFactory : ISqlAliasManagerFactory
+{
+    /// <summary>
+    ///     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.
+    /// </summary>
+    public SqlAliasManager Create()
+        => new();
+}

src/EFCore.Relational/Query/Internal/TpcTablesExpression.cs

@@ -109,11 +109,11 @@ protected override TableExpressionBase CreateWithAnnotations(IEnumerable<IAnnota
         => new TpcTablesExpression(Alias, EntityType, SelectExpressions, annotations);
 
     /// <inheritdoc />
-    public override TableExpressionBase Clone(ExpressionVisitor cloningExpressionVisitor)
+    public override TableExpressionBase Clone(string? alias, ExpressionVisitor cloningExpressionVisitor)
     {
         // Deep clone
         var subSelectExpressions = SelectExpressions.Select(cloningExpressionVisitor.Visit).ToList<SelectExpression>();
-        var newTpcTable = new TpcTablesExpression(Alias, EntityType, subSelectExpressions);
+        var newTpcTable = new TpcTablesExpression(alias, EntityType, subSelectExpressions);
         foreach (var annotation in GetAnnotations())
         {
             newTpcTable.AddAnnotation(annotation.Name, annotation.Value);

src/EFCore.Relational/Query/RelationalQueryCompilationContext.cs

@@ -28,6 +28,7 @@ public RelationalQueryCompilationContext(
     {
         RelationalDependencies = relationalDependencies;
         QuerySplittingBehavior = RelationalOptionsExtension.Extract(ContextOptions).QuerySplittingBehavior;
+        SqlAliasManager = relationalDependencies.SqlAliasManagerFactory.Create();
     }
 
     /// <summary>
@@ -41,4 +42,9 @@ public RelationalQueryCompilationContext(
     ///     will be used.
     /// </summary>
     public virtual QuerySplittingBehavior? QuerySplittingBehavior { get; internal set; }
+
+    /// <summary>
+    ///     A manager for SQL aliases, capable of generate uniquified table aliases.
+    /// </summary>
+    public virtual SqlAliasManager SqlAliasManager { get; }
 }

src/EFCore.Relational/Query/RelationalQueryCompilationContextDependencies.cs

@@ -45,7 +45,11 @@ public sealed record RelationalQueryCompilationContextDependencies
     ///     the constructor at any point in this process.
     /// </remarks>
     [EntityFrameworkInternal]
-    public RelationalQueryCompilationContextDependencies()
-    {
-    }
+    public RelationalQueryCompilationContextDependencies(ISqlAliasManagerFactory sqlAliasManagerFactory)
+        => SqlAliasManagerFactory = sqlAliasManagerFactory;
+
+    /// <summary>
+    ///     The current context.
+    /// </summary>
+    public ISqlAliasManagerFactory SqlAliasManagerFactory { get; init; }
 }

src/EFCore.Relational/Query/RelationalQueryTranslationPostprocessor.cs

@@ -11,6 +11,7 @@ namespace Microsoft.EntityFrameworkCore.Query;
 public class RelationalQueryTranslationPostprocessor : QueryTranslationPostprocessor
 {
     private readonly SqlTreePruner _pruner = new();
+    private readonly SqlAliasManager _sqlAliasManager;
     private readonly bool _useRelationalNulls;
 
     /// <summary>
@@ -22,10 +23,11 @@ public class RelationalQueryTranslationPostprocessor : QueryTranslationPostproce
     public RelationalQueryTranslationPostprocessor(
         QueryTranslationPostprocessorDependencies dependencies,
         RelationalQueryTranslationPostprocessorDependencies relationalDependencies,
-        QueryCompilationContext queryCompilationContext)
+        RelationalQueryCompilationContext queryCompilationContext)
         : base(dependencies, queryCompilationContext)
     {
         RelationalDependencies = relationalDependencies;
+        _sqlAliasManager = queryCompilationContext.SqlAliasManager;
         _useRelationalNulls = RelationalOptionsExtension.Extract(queryCompilationContext.ContextOptions).UseRelationalNulls;
     }
 
@@ -37,24 +39,24 @@ public RelationalQueryTranslationPostprocessor(
     /// <inheritdoc />
     public override Expression Process(Expression query)
     {
-        query = base.Process(query);
-        query = new SelectExpressionProjectionApplyingExpressionVisitor(
-            ((RelationalQueryCompilationContext)QueryCompilationContext).QuerySplittingBehavior).Visit(query);
-        query = Prune(query);
+        var query1 = base.Process(query);
+        var query2 = new SelectExpressionProjectionApplyingExpressionVisitor(
+            ((RelationalQueryCompilationContext)QueryCompilationContext).QuerySplittingBehavior).Visit(query1);
+        var query3 = Prune(query2);
+
+        // TODO: This - and all the verifications below - should happen after all visitors have run, including provider-specific ones.
+        var query4 = _sqlAliasManager.PostprocessAliases(query3);
 
 #if DEBUG
         // Verifies that all SelectExpression are marked as immutable after this point.
-        new SelectExpressionMutableVerifyingExpressionVisitor().Visit(query);
-        // Verifies that all table aliases are uniquely assigned without skipping over
-        // Which points to possible mutation of a SelectExpression being used in multiple places.
-        new TableAliasVerifyingExpressionVisitor().Visit(query);
+        new SelectExpressionMutableVerifyingExpressionVisitor().Visit(query4);
 #endif
 
-        query = new SqlExpressionSimplifyingExpressionVisitor(RelationalDependencies.SqlExpressionFactory, _useRelationalNulls)
-            .Visit(query);
-        query = new RelationalValueConverterCompensatingExpressionVisitor(RelationalDependencies.SqlExpressionFactory).Visit(query);
+        var query5 = new SqlExpressionSimplifyingExpressionVisitor(RelationalDependencies.SqlExpressionFactory, _useRelationalNulls)
+            .Visit(query4);
+        var query6 = new RelationalValueConverterCompensatingExpressionVisitor(RelationalDependencies.SqlExpressionFactory).Visit(query5);
 
-        return query;
+        return query6;
     }
 
     /// <summary>
@@ -85,99 +87,5 @@ private sealed class SelectExpressionMutableVerifyingExpressionVisitor : Express
             }
         }
     }
-
-    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:
-                    VerifyUniqueAliasInExpression(shapedQueryExpression.QueryExpression);
-                    Visit(shapedQueryExpression.QueryExpression);
-                    return shapedQueryExpression;
-
-                case RelationalSplitCollectionShaperExpression relationalSplitCollectionShaperExpression:
-                    VerifyUniqueAliasInExpression(relationalSplitCollectionShaperExpression.SelectExpression);
-                    Visit(relationalSplitCollectionShaperExpression.InnerShaper);
-                    return relationalSplitCollectionShaperExpression;
-
-                case NonQueryExpression nonQueryExpression:
-                    VerifyUniqueAliasInExpression(nonQueryExpression.Expression);
-                    return nonQueryExpression;
-
-                default:
-                    return base.Visit(expression);
-            }
-        }
-
-        private void VerifyUniqueAliasInExpression(Expression expression)
-            => _scopedVisitor.EntryPoint(expression);
-
-        private sealed class ScopedVisitor : ExpressionVisitor
-        {
-            private readonly HashSet<string> _usedAliases = new(StringComparer.OrdinalIgnoreCase);
-            private readonly HashSet<TableExpressionBase> _visitedTableExpressionBases = new(ReferenceEqualityComparer.Instance);
-
-            public Expression EntryPoint(Expression expression)
-            {
-                _usedAliases.Clear();
-                _visitedTableExpressionBases.Clear();
-
-                if (expression is SelectExpression selectExpression)
-                {
-                    Check.DebugAssert(selectExpression.RemovedAliases is not null, "RemovedAliases not set");
-                    foreach (var alias in selectExpression.RemovedAliases)
-                    {
-                        _usedAliases.Add(alias);
-                    }
-                }
-
-                var result = Visit(expression);
-
-                foreach (var group in _usedAliases.GroupBy(e => e[..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;
-            }
-        }
-    }
 #endif
 }