From 078b6776389cd0d1f84d849a98160cc4541e0580 Mon Sep 17 00:00:00 2001 From: Shay Rojansky Date: Sun, 14 Jan 2024 15:05:03 +0100 Subject: [PATCH] Make TableExpressionBase.Alias immutable And do some cleanup around table annotations Continues work from #32558 --- .../Properties/RelationalStrings.Designer.cs | 16 ++++- .../Properties/RelationalStrings.resx | 6 ++ .../Query/Internal/TpcTablesExpression.cs | 20 +++--- .../Query/QuerySqlGenerator.cs | 34 ++++++---- .../Query/SqlAliasManager.cs | 15 +---- .../SqlExpressions/CrossApplyExpression.cs | 8 +-- .../SqlExpressions/CrossJoinExpression.cs | 8 +-- .../Query/SqlExpressions/DeleteExpression.cs | 13 ++-- .../Query/SqlExpressions/ExceptExpression.cs | 12 ++-- .../Query/SqlExpressions/FromSqlExpression.cs | 22 +++---- .../SqlExpressions/InnerJoinExpression.cs | 10 +-- .../SqlExpressions/IntersectExpression.cs | 12 ++-- .../SqlExpressions/JoinExpressionBase.cs | 9 ++- .../SqlExpressions/LeftJoinExpression.cs | 10 +-- .../SqlExpressions/OuterApplyExpression.cs | 8 +-- .../PredicateJoinExpressionBase.cs | 2 +- .../SqlExpressions/SelectExpression.Helper.cs | 24 +++---- .../Query/SqlExpressions/SelectExpression.cs | 64 +++++++------------ .../Query/SqlExpressions/SetOperationBase.cs | 10 +-- .../Query/SqlExpressions/TableExpression.cs | 26 +++++--- .../SqlExpressions/TableExpressionBase.cs | 59 ++++++++++++----- .../TableValuedFunctionExpression.cs | 30 ++++----- .../Query/SqlExpressions/UnionExpression.cs | 12 ++-- .../Query/SqlExpressions/UpdateExpression.cs | 13 ++-- .../Query/SqlExpressions/ValuesExpression.cs | 33 +++++++--- .../Query/SqlNullabilityProcessor.cs | 2 +- src/EFCore.Relational/Query/SqlTreePruner.cs | 2 +- ...rchConditionConvertingExpressionVisitor.cs | 2 +- .../Internal/SqlServerOpenJsonExpression.cs | 4 ++ .../Internal/SqlServerQuerySqlGenerator.cs | 26 +++++--- .../Internal/JsonEachExpression.cs | 4 ++ 31 files changed, 297 insertions(+), 219 deletions(-) diff --git a/src/EFCore.Relational/Properties/RelationalStrings.Designer.cs b/src/EFCore.Relational/Properties/RelationalStrings.Designer.cs index 0ba115a6f7f..513e97990e3 100644 --- a/src/EFCore.Relational/Properties/RelationalStrings.Designer.cs +++ b/src/EFCore.Relational/Properties/RelationalStrings.Designer.cs @@ -67,6 +67,12 @@ public static string CannotProjectNullableComplexType(object? complexType) GetString("CannotProjectNullableComplexType", nameof(complexType)), complexType); + /// + /// Join expressions have no aliases; set the alias on the enclosed table expression. + /// + public static string CannotSetAliasOnJoin + => GetString("CannotSetAliasOnJoin"); + /// /// The query contained a new array expression containing non-constant elements, which could not be translated: '{newArrayExpression}'. /// @@ -1339,6 +1345,14 @@ public static string NestedAmbientTransactionError public static string NoActiveTransaction => GetString("NoActiveTransaction"); + /// + /// No alias is defined on table: '{table}' + /// + public static string NoAliasOnTable(object? table) + => string.Format( + GetString("NoAliasOnTable", nameof(table)), + table); + /// /// Cannot create a DbCommand for a non-relational query. /// @@ -2046,7 +2060,7 @@ public static string UnsupportedOperatorForSqlExpression(object? nodeType, objec nodeType, expressionType); /// - /// No relational type mapping can be found for property '{entity}.{property}' and the current provider doesn't specify a default store type for the properties of type '{clrType}'. + /// No relational type mapping can be found for property '{entity}.{property}' and the current provider doesn't specify a default store type for the properties of type '{clrType}'. /// public static string UnsupportedPropertyType(object? entity, object? property, object? clrType) => string.Format( diff --git a/src/EFCore.Relational/Properties/RelationalStrings.resx b/src/EFCore.Relational/Properties/RelationalStrings.resx index 71f1a37c368..4f0e31ca873 100644 --- a/src/EFCore.Relational/Properties/RelationalStrings.resx +++ b/src/EFCore.Relational/Properties/RelationalStrings.resx @@ -136,6 +136,9 @@ You are attempting to project out complex type '{complexType}' via an optional navigation; that is currently not supported. Either project out the complex type in a non-optional context, or project the containing entity type along with the complex type. + + Join expressions have no aliases; set the alias on the enclosed table expression. + The query contained a new array expression containing non-constant elements, which could not be translated: '{newArrayExpression}'. @@ -927,6 +930,9 @@ The connection does not have any active transactions. + + No alias is defined on table: '{table}'. + Cannot create a DbCommand for a non-relational query. diff --git a/src/EFCore.Relational/Query/Internal/TpcTablesExpression.cs b/src/EFCore.Relational/Query/Internal/TpcTablesExpression.cs index bc77a6690a3..3b5b269e582 100644 --- a/src/EFCore.Relational/Query/Internal/TpcTablesExpression.cs +++ b/src/EFCore.Relational/Query/Internal/TpcTablesExpression.cs @@ -34,7 +34,7 @@ private TpcTablesExpression( string? alias, IEntityType entityType, IReadOnlyList subSelectExpressions, - IEnumerable? annotations) + IReadOnlyDictionary? annotations) : base(alias, annotations) { EntityType = entityType; @@ -47,12 +47,8 @@ private TpcTablesExpression( /// 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. /// - [NotNull] - public override string? Alias - { - get => base.Alias!; - internal set => base.Alias = value; - } + public override string Alias + => base.Alias!; /// /// This is an internal API that supports the Entity Framework Core infrastructure and not subject to @@ -86,7 +82,7 @@ public TpcTablesExpression Prune(IReadOnlyList discriminatorValues) Check.DebugAssert(subSelectExpressions.Count > 0, "TPC must have at least 1 table selected."); - return new TpcTablesExpression(Alias, EntityType, subSelectExpressions, GetAnnotations()); + return new TpcTablesExpression(Alias, EntityType, subSelectExpressions, Annotations); } /// @@ -105,8 +101,12 @@ protected override Expression VisitChildren(ExpressionVisitor visitor) /// any release. You should only use it directly in your code with extreme caution and knowing that /// doing so can result in application failures when updating to a new Entity Framework Core release. /// - protected override TableExpressionBase CreateWithAnnotations(IEnumerable annotations) - => new TpcTablesExpression(Alias, EntityType, SelectExpressions, annotations); + protected override TpcTablesExpression WithAnnotations(IReadOnlyDictionary annotations) + => new(Alias, EntityType, SelectExpressions, annotations); + + /// + public override TpcTablesExpression WithAlias(string newAlias) + => new(newAlias, EntityType, SelectExpressions, Annotations); /// public override TableExpressionBase Clone(string? alias, ExpressionVisitor cloningExpressionVisitor) diff --git a/src/EFCore.Relational/Query/QuerySqlGenerator.cs b/src/EFCore.Relational/Query/QuerySqlGenerator.cs index ed574161e0e..a05d78e2328 100644 --- a/src/EFCore.Relational/Query/QuerySqlGenerator.cs +++ b/src/EFCore.Relational/Query/QuerySqlGenerator.cs @@ -191,14 +191,17 @@ protected override Expression VisitDelete(DeleteExpression deleteExpression) { var selectExpression = deleteExpression.SelectExpression; - if (selectExpression.Offset == null - && selectExpression.Limit == null - && selectExpression.Having == null - && selectExpression.Orderings.Count == 0 - && selectExpression.GroupBy.Count == 0 - && selectExpression.Tables.Count == 1 - && selectExpression.Tables[0] == deleteExpression.Table - && selectExpression.Projection.Count == 0) + if (selectExpression is + { + Tables: [var table], + GroupBy: [], + Having: null, + Projection: [], + Orderings: [], + Offset: null, + Limit: null + } + && table.Equals(deleteExpression.Table)) { _relationalCommandBuilder.Append("DELETE FROM "); Visit(deleteExpression.Table); @@ -1349,12 +1352,15 @@ protected override Expression VisitUpdate(UpdateExpression updateExpression) { var selectExpression = updateExpression.SelectExpression; - if (selectExpression.Offset == null - && selectExpression.Limit == null - && selectExpression.Having == null - && selectExpression.Orderings.Count == 0 - && selectExpression.GroupBy.Count == 0 - && selectExpression.Projection.Count == 0 + if (selectExpression is + { + Offset: null, + Limit: null, + Having: null, + Orderings: [], + GroupBy: [], + Projection: [], + } && (selectExpression.Tables.Count == 1 || !ReferenceEquals(selectExpression.Tables[0], updateExpression.Table) || selectExpression.Tables[1] is InnerJoinExpression diff --git a/src/EFCore.Relational/Query/SqlAliasManager.cs b/src/EFCore.Relational/Query/SqlAliasManager.cs index d3ed60f43a2..ba4b4def770 100644 --- a/src/EFCore.Relational/Query/SqlAliasManager.cs +++ b/src/EFCore.Relational/Query/SqlAliasManager.cs @@ -168,8 +168,6 @@ protected override Expression VisitExtension(Expression node) private sealed class TableAliasRewriter(IReadOnlyDictionary aliasRewritingMap) : ExpressionVisitor { - private readonly HashSet _visitedTables = new(ReferenceEqualityComparer.Instance); - internal static Expression Rewrite(Expression expression, IReadOnlyDictionary aliasRewritingMap) => new TableAliasRewriter(aliasRewritingMap).Visit(expression); @@ -182,20 +180,9 @@ protected override Expression VisitExtension(Expression node) // Note that this skips joins (which wrap the table that has the actual alias), as well as the top-level select case TableExpressionBase { Alias: string alias } table: - // TODO: Needed only because TableExpressionBase is still mutable with regards to its alias - remove after that's gone. - if (!_visitedTables.Add(table)) - { - return table; - } - if (aliasRewritingMap.TryGetValue(alias, out var newAlias)) { - // TODO: TableExpressionBase is still mutable with regards to its alias - this needs to change. - // TODO: This visitor needs to replace the table in the usual way; but we don't currently have a good way of - // TODO: recreating a new TableExpressionBase implementation with a different alias. - // TODO: Either add a mandatory, abstract non-destructive ChangeAlias to TableExpressionBase, or ideally, - // TODO: refactor things to split the alias out of TableExpressionBase altogether. - table.Alias = newAlias; + table = table.WithAlias(newAlias); } return base.VisitExtension(table); diff --git a/src/EFCore.Relational/Query/SqlExpressions/CrossApplyExpression.cs b/src/EFCore.Relational/Query/SqlExpressions/CrossApplyExpression.cs index 3263c42e759..6b13a8fe4dc 100644 --- a/src/EFCore.Relational/Query/SqlExpressions/CrossApplyExpression.cs +++ b/src/EFCore.Relational/Query/SqlExpressions/CrossApplyExpression.cs @@ -23,7 +23,7 @@ public CrossApplyExpression(TableExpressionBase table) { } - private CrossApplyExpression(TableExpressionBase table, IEnumerable? annotations) + private CrossApplyExpression(TableExpressionBase table, IReadOnlyDictionary? annotations) : base(table, prunable: false, annotations) { } @@ -40,12 +40,12 @@ protected override Expression VisitChildren(ExpressionVisitor visitor) /// This expression if no children changed, or an expression with the updated children. public override CrossApplyExpression Update(TableExpressionBase table) => table != Table - ? new CrossApplyExpression(table, GetAnnotations()) + ? new CrossApplyExpression(table, Annotations) : this; /// - protected override TableExpressionBase CreateWithAnnotations(IEnumerable annotations) - => new CrossApplyExpression(Table, GetAnnotations()); + protected override CrossApplyExpression WithAnnotations(IReadOnlyDictionary annotations) + => new(Table, annotations); /// protected override void Print(ExpressionPrinter expressionPrinter) diff --git a/src/EFCore.Relational/Query/SqlExpressions/CrossJoinExpression.cs b/src/EFCore.Relational/Query/SqlExpressions/CrossJoinExpression.cs index 80b6de42e38..9b5a58de124 100644 --- a/src/EFCore.Relational/Query/SqlExpressions/CrossJoinExpression.cs +++ b/src/EFCore.Relational/Query/SqlExpressions/CrossJoinExpression.cs @@ -23,7 +23,7 @@ public CrossJoinExpression(TableExpressionBase table) { } - private CrossJoinExpression(TableExpressionBase table, IEnumerable? annotations) + private CrossJoinExpression(TableExpressionBase table, IReadOnlyDictionary? annotations) : base(table, prunable: false, annotations) { } @@ -40,12 +40,12 @@ protected override Expression VisitChildren(ExpressionVisitor visitor) /// This expression if no children changed, or an expression with the updated children. public override CrossJoinExpression Update(TableExpressionBase table) => table != Table - ? new CrossJoinExpression(table, GetAnnotations()) + ? new CrossJoinExpression(table, Annotations) : this; /// - protected override TableExpressionBase CreateWithAnnotations(IEnumerable annotations) - => new CrossJoinExpression(Table, annotations); + protected override CrossJoinExpression WithAnnotations(IReadOnlyDictionary annotations) + => new(Table, Annotations); /// protected override void Print(ExpressionPrinter expressionPrinter) diff --git a/src/EFCore.Relational/Query/SqlExpressions/DeleteExpression.cs b/src/EFCore.Relational/Query/SqlExpressions/DeleteExpression.cs index bfdb7c2bc26..80bb41ebbc2 100644 --- a/src/EFCore.Relational/Query/SqlExpressions/DeleteExpression.cs +++ b/src/EFCore.Relational/Query/SqlExpressions/DeleteExpression.cs @@ -64,20 +64,21 @@ public override ExpressionType NodeType protected override Expression VisitChildren(ExpressionVisitor visitor) { var selectExpression = (SelectExpression)visitor.Visit(SelectExpression); - - return Update(selectExpression); + var table = (TableExpression)visitor.Visit(Table); + return Update(table, selectExpression); } /// /// 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. /// This expression if no children changed, or an expression with the updated children. - public DeleteExpression Update(SelectExpression selectExpression) - => selectExpression != SelectExpression - ? new DeleteExpression(Table, selectExpression, Tags) - : this; + public DeleteExpression Update(TableExpression table, SelectExpression selectExpression) + => table == Table && selectExpression == SelectExpression + ? this + : new DeleteExpression(table, selectExpression, Tags); /// public void Print(ExpressionPrinter expressionPrinter) diff --git a/src/EFCore.Relational/Query/SqlExpressions/ExceptExpression.cs b/src/EFCore.Relational/Query/SqlExpressions/ExceptExpression.cs index 354c2bf82b4..cb752a97bfd 100644 --- a/src/EFCore.Relational/Query/SqlExpressions/ExceptExpression.cs +++ b/src/EFCore.Relational/Query/SqlExpressions/ExceptExpression.cs @@ -35,7 +35,7 @@ private ExceptExpression( SelectExpression source1, SelectExpression source2, bool distinct, - IEnumerable? annotations) + IReadOnlyDictionary? annotations) : base(alias, source1, source2, distinct, annotations) { } @@ -58,12 +58,12 @@ protected override Expression VisitChildren(ExpressionVisitor visitor) /// This expression if no children changed, or an expression with the updated children. public override ExceptExpression Update(SelectExpression source1, SelectExpression source2) => source1 != Source1 || source2 != Source2 - ? new ExceptExpression(Alias, source1, source2, IsDistinct, GetAnnotations()) + ? new ExceptExpression(Alias, source1, source2, IsDistinct, Annotations) : this; /// - protected override TableExpressionBase CreateWithAnnotations(IEnumerable annotations) - => new ExceptExpression(Alias, Source1, Source2, IsDistinct, annotations); + protected override ExceptExpression WithAnnotations(IReadOnlyDictionary annotations) + => new(Alias, Source1, Source2, IsDistinct, annotations); /// public override TableExpressionBase Clone(string? alias, ExpressionVisitor cloningExpressionVisitor) @@ -73,6 +73,10 @@ public override TableExpressionBase Clone(string? alias, ExpressionVisitor cloni (SelectExpression)cloningExpressionVisitor.Visit(Source2), IsDistinct); + /// + public override ExceptExpression WithAlias(string newAlias) + => new(newAlias, Source1, Source2, IsDistinct); + /// protected override void Print(ExpressionPrinter expressionPrinter) { diff --git a/src/EFCore.Relational/Query/SqlExpressions/FromSqlExpression.cs b/src/EFCore.Relational/Query/SqlExpressions/FromSqlExpression.cs index ef119d0d332..97ac89511dd 100644 --- a/src/EFCore.Relational/Query/SqlExpressions/FromSqlExpression.cs +++ b/src/EFCore.Relational/Query/SqlExpressions/FromSqlExpression.cs @@ -54,7 +54,7 @@ private FromSqlExpression( ITableBase? tableBase, string sql, Expression arguments, - IEnumerable? annotations) + IReadOnlyDictionary? annotations) : base(alias, annotations) { Table = tableBase; @@ -65,12 +65,8 @@ private FromSqlExpression( /// /// The alias assigned to this table source. /// - [NotNull] - public override string? Alias - { - get => base.Alias!; - internal set => base.Alias = value; - } + public override string Alias + => base.Alias!; /// /// The user-provided custom SQL for the table source. @@ -95,12 +91,16 @@ public override string? Alias /// This expression if no children changed, or an expression with the updated children. public virtual FromSqlExpression Update(Expression arguments) => arguments != Arguments - ? new FromSqlExpression(Alias, Table, Sql, arguments, GetAnnotations()) + ? new FromSqlExpression(Alias, Table, Sql, arguments, Annotations) : this; /// - protected override TableExpressionBase CreateWithAnnotations(IEnumerable annotations) - => new FromSqlExpression(Alias, Table, Sql, Arguments, annotations); + protected override FromSqlExpression WithAnnotations(IReadOnlyDictionary annotations) + => new(Alias, Table, Sql, Arguments, annotations); + + /// + public override FromSqlExpression WithAlias(string newAlias) + => new(newAlias, Table, Sql, Arguments, Annotations); /// protected override Expression VisitChildren(ExpressionVisitor visitor) @@ -108,7 +108,7 @@ protected override Expression VisitChildren(ExpressionVisitor visitor) /// public override TableExpressionBase Clone(string? alias, ExpressionVisitor cloningVisitor) - => new FromSqlExpression(alias!, Table, Sql, Arguments, GetAnnotations()); + => new FromSqlExpression(alias!, Table, Sql, Arguments, Annotations); /// protected override void Print(ExpressionPrinter expressionPrinter) diff --git a/src/EFCore.Relational/Query/SqlExpressions/InnerJoinExpression.cs b/src/EFCore.Relational/Query/SqlExpressions/InnerJoinExpression.cs index 54ba76bf605..5373bee4a00 100644 --- a/src/EFCore.Relational/Query/SqlExpressions/InnerJoinExpression.cs +++ b/src/EFCore.Relational/Query/SqlExpressions/InnerJoinExpression.cs @@ -29,7 +29,7 @@ private InnerJoinExpression( TableExpressionBase table, SqlExpression joinPredicate, bool prunable, - IEnumerable? annotations) + IReadOnlyDictionary? annotations) : base(table, joinPredicate, prunable, annotations) { } @@ -43,7 +43,7 @@ private InnerJoinExpression( /// This expression if no children changed, or an expression with the updated children. public override InnerJoinExpression Update(TableExpressionBase table, SqlExpression joinPredicate) => table != Table || joinPredicate != JoinPredicate - ? new InnerJoinExpression(table, joinPredicate, IsPrunable, GetAnnotations()) + ? new InnerJoinExpression(table, joinPredicate, IsPrunable, Annotations) : this; /// @@ -54,12 +54,12 @@ public override InnerJoinExpression Update(TableExpressionBase table, SqlExpress /// This expression if no children changed, or an expression with the updated children. public override InnerJoinExpression Update(TableExpressionBase table) => table != Table - ? new InnerJoinExpression(table, JoinPredicate, IsPrunable, GetAnnotations()) + ? new InnerJoinExpression(table, JoinPredicate, IsPrunable, Annotations) : this; /// - protected override TableExpressionBase CreateWithAnnotations(IEnumerable annotations) - => new InnerJoinExpression(Table, JoinPredicate, IsPrunable, annotations); + protected override InnerJoinExpression WithAnnotations(IReadOnlyDictionary annotations) + => new(Table, JoinPredicate, IsPrunable, annotations); /// protected override void Print(ExpressionPrinter expressionPrinter) diff --git a/src/EFCore.Relational/Query/SqlExpressions/IntersectExpression.cs b/src/EFCore.Relational/Query/SqlExpressions/IntersectExpression.cs index 501563f5cc0..1ce1c89b91f 100644 --- a/src/EFCore.Relational/Query/SqlExpressions/IntersectExpression.cs +++ b/src/EFCore.Relational/Query/SqlExpressions/IntersectExpression.cs @@ -35,7 +35,7 @@ private IntersectExpression( SelectExpression source1, SelectExpression source2, bool distinct, - IEnumerable? annotations) + IReadOnlyDictionary? annotations) : base(alias, source1, source2, distinct, annotations) { } @@ -58,12 +58,12 @@ protected override Expression VisitChildren(ExpressionVisitor visitor) /// This expression if no children changed, or an expression with the updated children. public override IntersectExpression Update(SelectExpression source1, SelectExpression source2) => source1 != Source1 || source2 != Source2 - ? new IntersectExpression(Alias, source1, source2, IsDistinct, GetAnnotations()) + ? new IntersectExpression(Alias, source1, source2, IsDistinct, Annotations) : this; /// - protected override TableExpressionBase CreateWithAnnotations(IEnumerable annotations) - => new IntersectExpression(Alias, Source1, Source2, IsDistinct, annotations); + protected override IntersectExpression WithAnnotations(IReadOnlyDictionary annotations) + => new(Alias, Source1, Source2, IsDistinct, annotations); /// public override TableExpressionBase Clone(string? alias, ExpressionVisitor cloningExpressionVisitor) @@ -73,6 +73,10 @@ public override TableExpressionBase Clone(string? alias, ExpressionVisitor cloni (SelectExpression)cloningExpressionVisitor.Visit(Source2), IsDistinct); + /// + public override IntersectExpression WithAlias(string newAlias) + => new(newAlias, Source1, Source2, IsDistinct); + /// protected override void Print(ExpressionPrinter expressionPrinter) { diff --git a/src/EFCore.Relational/Query/SqlExpressions/JoinExpressionBase.cs b/src/EFCore.Relational/Query/SqlExpressions/JoinExpressionBase.cs index 96658271fa7..027494f77b5 100644 --- a/src/EFCore.Relational/Query/SqlExpressions/JoinExpressionBase.cs +++ b/src/EFCore.Relational/Query/SqlExpressions/JoinExpressionBase.cs @@ -20,7 +20,7 @@ public abstract class JoinExpressionBase : TableExpressionBase /// A table source to join with. /// Whether this join expression may be pruned if nothing references a column on it. /// A collection of annotations associated with this expression. - protected JoinExpressionBase(TableExpressionBase table, bool prunable, IEnumerable? annotations = null) + protected JoinExpressionBase(TableExpressionBase table, bool prunable, IReadOnlyDictionary? annotations = null) : base(alias: null, annotations) { Table = table; @@ -52,6 +52,13 @@ protected JoinExpressionBase(TableExpressionBase table, bool prunable, IEnumerab public override TableExpressionBase Clone(string? alias, ExpressionVisitor cloningExpressionVisitor) => (TableExpressionBase)VisitChildren(cloningExpressionVisitor); + /// + /// The implementation of for join expressions always throws, since the alias on joins is always + /// . Set the alias on the enclosed table expression instead. + /// + public override TableExpressionBase WithAlias(string newAlias) + => throw new InvalidOperationException(RelationalStrings.CannotSetAliasOnJoin); + /// /// This is an internal API that supports the Entity Framework Core infrastructure and not subject to /// the same compatibility standards as public APIs. It may be changed or removed without notice in diff --git a/src/EFCore.Relational/Query/SqlExpressions/LeftJoinExpression.cs b/src/EFCore.Relational/Query/SqlExpressions/LeftJoinExpression.cs index 24c1725f9d0..1fc5dbef91a 100644 --- a/src/EFCore.Relational/Query/SqlExpressions/LeftJoinExpression.cs +++ b/src/EFCore.Relational/Query/SqlExpressions/LeftJoinExpression.cs @@ -29,7 +29,7 @@ private LeftJoinExpression( TableExpressionBase table, SqlExpression joinPredicate, bool prunable, - IEnumerable? annotations = null) + IReadOnlyDictionary? annotations = null) : base(table, joinPredicate, prunable, annotations) { } @@ -43,7 +43,7 @@ private LeftJoinExpression( /// This expression if no children changed, or an expression with the updated children. public override LeftJoinExpression Update(TableExpressionBase table, SqlExpression joinPredicate) => table != Table || joinPredicate != JoinPredicate - ? new LeftJoinExpression(table, joinPredicate, IsPrunable, GetAnnotations()) + ? new LeftJoinExpression(table, joinPredicate, IsPrunable, Annotations) : this; /// @@ -54,12 +54,12 @@ public override LeftJoinExpression Update(TableExpressionBase table, SqlExpressi /// This expression if no children changed, or an expression with the updated children. public override LeftJoinExpression Update(TableExpressionBase table) => table != Table - ? new LeftJoinExpression(table, JoinPredicate, IsPrunable, GetAnnotations()) + ? new LeftJoinExpression(table, JoinPredicate, IsPrunable, Annotations) : this; /// - protected override TableExpressionBase CreateWithAnnotations(IEnumerable annotations) - => new LeftJoinExpression(Table, JoinPredicate, IsPrunable, annotations); + protected override LeftJoinExpression WithAnnotations(IReadOnlyDictionary annotations) + => new(Table, JoinPredicate, IsPrunable, annotations); /// protected override void Print(ExpressionPrinter expressionPrinter) diff --git a/src/EFCore.Relational/Query/SqlExpressions/OuterApplyExpression.cs b/src/EFCore.Relational/Query/SqlExpressions/OuterApplyExpression.cs index 74940bebbfe..6a64ce346d8 100644 --- a/src/EFCore.Relational/Query/SqlExpressions/OuterApplyExpression.cs +++ b/src/EFCore.Relational/Query/SqlExpressions/OuterApplyExpression.cs @@ -23,7 +23,7 @@ public OuterApplyExpression(TableExpressionBase table) { } - private OuterApplyExpression(TableExpressionBase table, IEnumerable? annotations) + private OuterApplyExpression(TableExpressionBase table, IReadOnlyDictionary? annotations) : base(table, prunable: false, annotations) { } @@ -40,12 +40,12 @@ protected override Expression VisitChildren(ExpressionVisitor visitor) /// This expression if no children changed, or an expression with the updated children. public override OuterApplyExpression Update(TableExpressionBase table) => table != Table - ? new OuterApplyExpression(table, GetAnnotations()) + ? new OuterApplyExpression(table, Annotations) : this; /// - protected override TableExpressionBase CreateWithAnnotations(IEnumerable annotations) - => new OuterApplyExpression(Table, annotations); + protected override OuterApplyExpression WithAnnotations(IReadOnlyDictionary annotations) + => new(Table, annotations); /// protected override void Print(ExpressionPrinter expressionPrinter) diff --git a/src/EFCore.Relational/Query/SqlExpressions/PredicateJoinExpressionBase.cs b/src/EFCore.Relational/Query/SqlExpressions/PredicateJoinExpressionBase.cs index 815e596fac2..ca361689f3e 100644 --- a/src/EFCore.Relational/Query/SqlExpressions/PredicateJoinExpressionBase.cs +++ b/src/EFCore.Relational/Query/SqlExpressions/PredicateJoinExpressionBase.cs @@ -25,7 +25,7 @@ protected PredicateJoinExpressionBase( TableExpressionBase table, SqlExpression joinPredicate, bool prunable, - IEnumerable? annotations = null) + IReadOnlyDictionary? annotations = null) : base(table, prunable, annotations) { JoinPredicate = joinPredicate; diff --git a/src/EFCore.Relational/Query/SqlExpressions/SelectExpression.Helper.cs b/src/EFCore.Relational/Query/SqlExpressions/SelectExpression.Helper.cs index 146386f8d8c..3349ac8dba9 100644 --- a/src/EFCore.Relational/Query/SqlExpressions/SelectExpression.Helper.cs +++ b/src/EFCore.Relational/Query/SqlExpressions/SelectExpression.Helper.cs @@ -469,10 +469,9 @@ public TpcTableExpressionRemovingExpressionVisitor(SqlAliasManager sqlAliasManag // Any non-column projection means some composition which cannot be removed && selectExpression.Projection.All(e => e.Expression is ColumnExpression); - foreach (var kvp in selectExpression._tpcDiscriminatorValues) + foreach (var (tpcTablesExpression, (_, discriminatorValues)) in selectExpression._tpcDiscriminatorValues) { - var tpcTablesExpression = kvp.Key; - var subSelectExpressions = tpcTablesExpression.Prune(kvp.Value.Item2).SelectExpressions; + var subSelectExpressions = tpcTablesExpression.Prune(discriminatorValues).SelectExpressions; var firstSelectExpression = subSelectExpressions[0]; // There will be at least one. int[]? reindexingMap = null; @@ -534,27 +533,28 @@ public TpcTableExpressionRemovingExpressionVisitor(SqlAliasManager sqlAliasManag if (identitySelect) { - result.Alias = selectExpression.Alias; if (selectExpression.Alias == null) { // If top-level them copy over bindings for shaper result._projectionMapping = selectExpression._projectionMapping; result._clientProjections = selectExpression._clientProjections; } + else + { + result = result.WithAlias(selectExpression.Alias); + } // Since identity select implies only 1 table so we can return without worrying about another iteration. // Identity select shouldn't require base visit. return result; } - { - result.Alias = tpcTablesExpression.Alias; - var tableIndex = - selectExpression._tables.FindIndex(teb => ReferenceEquals(teb.UnwrapJoin(), tpcTablesExpression)); - var table = selectExpression._tables[tableIndex]; - selectExpression._tables[tableIndex] = (TableExpressionBase)ReplacingExpressionVisitor.Replace( - tpcTablesExpression, result, table); - } + result = result.WithAlias(tpcTablesExpression.Alias); + var tableIndex = + selectExpression._tables.FindIndex(teb => ReferenceEquals(teb.UnwrapJoin(), tpcTablesExpression)); + var table = selectExpression._tables[tableIndex]; + selectExpression._tables[tableIndex] = (TableExpressionBase)ReplacingExpressionVisitor.Replace( + tpcTablesExpression, result, table); } selectExpression._tpcDiscriminatorValues.Clear(); diff --git a/src/EFCore.Relational/Query/SqlExpressions/SelectExpression.cs b/src/EFCore.Relational/Query/SqlExpressions/SelectExpression.cs index 9e26bbdcbd9..520b7059687 100644 --- a/src/EFCore.Relational/Query/SqlExpressions/SelectExpression.cs +++ b/src/EFCore.Relational/Query/SqlExpressions/SelectExpression.cs @@ -46,8 +46,6 @@ public sealed partial class SelectExpression : TableExpressionBase private readonly List _aliasForClientProjections = []; private CloningExpressionVisitor? _cloningExpressionVisitor; - private SortedDictionary? _annotations; - // We need to remember identifiers before GroupBy in case it is final GroupBy and element selector has a collection // This state doesn't need to propagate // It should be only at top-level otherwise GroupBy won't be final operator. @@ -61,24 +59,14 @@ private SelectExpression( List tables, List groupBy, List orderings, - IEnumerable annotations, + IReadOnlyDictionary? annotations, SqlAliasManager sqlAliasManager) - : base(alias) + : base(alias, annotations) { _projection = projections; _tables = tables; _groupBy = groupBy; _orderings = orderings; - - if (annotations != null) - { - _annotations = new SortedDictionary(); - foreach (var annotation in annotations) - { - _annotations[annotation.Name] = annotation; - } - } - _sqlAliasManager = sqlAliasManager; } @@ -2466,7 +2454,7 @@ private void ApplySetOperation( { // TODO: Introduce clone method? See issue#24460 var select1 = new SelectExpression( - alias: null, projections: [], _tables.ToList(), _groupBy.ToList(), _orderings.ToList(), GetAnnotations(), _sqlAliasManager) + alias: null, projections: [], _tables.ToList(), _groupBy.ToList(), _orderings.ToList(), Annotations, _sqlAliasManager) { IsDistinct = IsDistinct, Predicate = Predicate, @@ -3936,7 +3924,7 @@ private SqlRemappingVisitor PushdownIntoSubqueryInternal(bool liftOrderings = tr _sqlAliasManager.GenerateTableAlias(_tables is [{ Alias: string singleTableAlias }] ? singleTableAlias : "subquery"); var subquery = new SelectExpression( - subqueryAlias, [], _tables.ToList(), _groupBy.ToList(), _orderings.ToList(), GetAnnotations(), _sqlAliasManager) + subqueryAlias, [], _tables.ToList(), _groupBy.ToList(), _orderings.ToList(), Annotations, _sqlAliasManager) { IsDistinct = IsDistinct, Predicate = Predicate, @@ -4318,7 +4306,7 @@ public override TableExpressionBase Clone(string? alias, ExpressionVisitor cloni var limit = (SqlExpression?)cloningExpressionVisitor.Visit(Limit); var newSelectExpression = new SelectExpression( - alias, newProjections, newTables, newGroupBy, newOrderings, GetAnnotations(), _sqlAliasManager) + alias, newProjections, newTables, newGroupBy, newOrderings, Annotations, _sqlAliasManager) { Predicate = predicate, Having = havingExpression, @@ -4776,7 +4764,7 @@ protected override Expression VisitChildren(ExpressionVisitor visitor) if (changed) { var newSelectExpression = new SelectExpression( - Alias, newProjections, newTables, newGroupBy, newOrderings, GetAnnotations(), _sqlAliasManager) + Alias, newProjections, newTables, newGroupBy, newOrderings, Annotations, _sqlAliasManager) { _clientProjections = _clientProjections, _projectionMapping = _projectionMapping, @@ -4881,7 +4869,7 @@ public SelectExpression Update( // TODO: This always creates a new expression. It should check if anything changed instead (#31276), allowing us to remove "changed" // tracking from calling code. var newSelectExpression = new SelectExpression( - Alias, projections.ToList(), tables.ToList(), groupBy.ToList(), orderings.ToList(), GetAnnotations(), _sqlAliasManager) + Alias, projections.ToList(), tables.ToList(), groupBy.ToList(), orderings.ToList(), Annotations, _sqlAliasManager) { _projectionMapping = projectionMapping, _clientProjections = _clientProjections.ToList(), @@ -4901,34 +4889,28 @@ public SelectExpression Update( } /// - protected override TableExpressionBase CreateWithAnnotations(IEnumerable annotations) - => throw new NotImplementedException("inconceivable"); + protected override SelectExpression WithAnnotations(IReadOnlyDictionary annotations) + => throw new UnreachableException("inconceivable"); /// - public override TableExpressionBase AddAnnotation(string name, object? value) + public override SelectExpression WithAlias(string newAlias) { - var oldAnnotation = FindAnnotation(name); - if (oldAnnotation != null) - { - return Equals(oldAnnotation.Value, value) - ? this - : throw new InvalidOperationException(CoreStrings.DuplicateAnnotation(name, this.Print())); - } + Check.DebugAssert(!_mutable, "Can't change alias on mutable SelectExpression"); - _annotations ??= new SortedDictionary(); - _annotations[name] = new Annotation(name, value); - - return this; + return new SelectExpression(newAlias, _projection, _tables, _groupBy, _orderings, Annotations, _sqlAliasManager) + { + _projectionMapping = _projectionMapping, + _clientProjections = _clientProjections.ToList(), + Predicate = Predicate, + Having = Having, + Offset = Offset, + Limit = Limit, + IsDistinct = IsDistinct, + Tags = Tags, + _mutable = false + }; } - /// - public override IAnnotation? FindAnnotation(string name) - => _annotations?.GetValueOrDefault(name); - - /// - public override IEnumerable GetAnnotations() - => _annotations?.Values ?? Enumerable.Empty(); - /// protected override void Print(ExpressionPrinter expressionPrinter) { diff --git a/src/EFCore.Relational/Query/SqlExpressions/SetOperationBase.cs b/src/EFCore.Relational/Query/SqlExpressions/SetOperationBase.cs index aa324cf1c97..96ed42285ac 100644 --- a/src/EFCore.Relational/Query/SqlExpressions/SetOperationBase.cs +++ b/src/EFCore.Relational/Query/SqlExpressions/SetOperationBase.cs @@ -45,7 +45,7 @@ protected SetOperationBase( SelectExpression source1, SelectExpression source2, bool distinct, - IEnumerable? annotations) + IReadOnlyDictionary? annotations) : base(alias, annotations) { IsDistinct = distinct; @@ -56,12 +56,8 @@ protected SetOperationBase( /// /// The alias assigned to this table source. /// - [NotNull] - public override string? Alias - { - get => base.Alias!; - internal set => base.Alias = value; - } + public override string Alias + => base.Alias!; /// /// The bool value indicating whether result will remove duplicate rows. diff --git a/src/EFCore.Relational/Query/SqlExpressions/TableExpression.cs b/src/EFCore.Relational/Query/SqlExpressions/TableExpression.cs index 5a1ad00fa4c..7db678bfa20 100644 --- a/src/EFCore.Relational/Query/SqlExpressions/TableExpression.cs +++ b/src/EFCore.Relational/Query/SqlExpressions/TableExpression.cs @@ -20,7 +20,7 @@ internal TableExpression(string alias, ITableBase table) { } - private TableExpression(string alias, ITableBase table, IEnumerable? annotations) + private TableExpression(string alias, ITableBase table, IReadOnlyDictionary? annotations) : base(alias, annotations) { Name = table.Name; @@ -49,10 +49,6 @@ public override string Alias /// public ITableBase Table { get; } - /// - protected override TableExpressionBase CreateWithAnnotations(IEnumerable annotations) - => new TableExpression(Alias, Table, annotations); - /// ITableBase ITableBasedExpression.Table => Table; @@ -73,12 +69,26 @@ protected override void Print(ExpressionPrinter expressionPrinter) /// public override TableExpressionBase Clone(string? alias, ExpressionVisitor cloningExpressionVisitor) - => new TableExpression(alias!, Table, GetAnnotations()); + => new TableExpression(alias!, Table, Annotations); + + /// + protected override TableExpression WithAnnotations(IReadOnlyDictionary annotations) + => new(Alias, Table, annotations); + + /// + public override TableExpression WithAlias(string newAlias) + => new(newAlias, Table, Annotations); /// public override bool Equals(object? obj) - // This should be reference equal only. - => obj != null && ReferenceEquals(this, obj); + => obj != null + && (ReferenceEquals(this, obj) + || obj is TableExpression fromSqlExpression + && Equals(fromSqlExpression)); + + private bool Equals(TableExpression fromSqlExpression) + => base.Equals(fromSqlExpression) + && Table == fromSqlExpression.Table; /// public override int GetHashCode() diff --git a/src/EFCore.Relational/Query/SqlExpressions/TableExpressionBase.cs b/src/EFCore.Relational/Query/SqlExpressions/TableExpressionBase.cs index a977be6605a..fea3f911f98 100644 --- a/src/EFCore.Relational/Query/SqlExpressions/TableExpressionBase.cs +++ b/src/EFCore.Relational/Query/SqlExpressions/TableExpressionBase.cs @@ -1,6 +1,8 @@ // Licensed to the .NET Foundation under one or more agreements. // The .NET Foundation licenses this file to you under the MIT license. +using System.Collections.Immutable; + namespace Microsoft.EntityFrameworkCore.Query.SqlExpressions; /// @@ -15,13 +17,16 @@ namespace Microsoft.EntityFrameworkCore.Query.SqlExpressions; [DebuggerDisplay("{Microsoft.EntityFrameworkCore.Query.ExpressionPrinter.Print(this), nq}")] public abstract class TableExpressionBase : Expression, IPrintableExpression { - private readonly IReadOnlyDictionary? _annotations; + /// + /// An indexed collection of annotations associated with this table expression. + /// + protected virtual IReadOnlyDictionary? Annotations { get; } /// /// Creates a new instance of the class. /// /// A string alias for the table source. - /// A collection of annotations associated with this expression. + /// A collection of annotations associated with this table expression. protected TableExpressionBase(string? alias, IEnumerable? annotations = null) { Alias = alias; @@ -34,14 +39,25 @@ protected TableExpressionBase(string? alias, IEnumerable? annotatio dictionary[annotation.Name] = annotation; } - _annotations = dictionary; + Annotations = dictionary; } } + /// + /// Creates a new instance of the class. + /// + /// A string alias for the table source. + /// A collection of annotations associated with this expression. + protected TableExpressionBase(string? alias, IReadOnlyDictionary? annotations) + { + Alias = alias; + Annotations = annotations; + } + /// /// The alias assigned to this table source. /// - public virtual string? Alias { get; internal set; } + public virtual string? Alias { get; } /// protected override Expression VisitChildren(ExpressionVisitor visitor) @@ -63,6 +79,12 @@ public sealed override ExpressionType NodeType /// A new object that is a copy of this instance. public abstract TableExpressionBase Clone(string? alias, ExpressionVisitor cloningExpressionVisitor); + /// + /// Returns a copy of the current with the new provided alias. + /// + /// The alias to apply to the returned . + public abstract TableExpressionBase WithAlias(string newAlias); + /// /// Creates a printable string representation of the given expression using . /// @@ -100,7 +122,7 @@ private bool Equals(TableExpressionBase tableExpressionBase) /// public override int GetHashCode() - => 0; + => Alias?.GetHashCode() ?? 0; /// /// Adds an annotation to this object. Throws if an annotation with the specified name already exists. @@ -118,9 +140,20 @@ public virtual TableExpressionBase AddAnnotation(string name, object? value) : throw new InvalidOperationException(CoreStrings.DuplicateAnnotation(name, this.Print())); } - var annotation = new Annotation(name, value); - return CreateWithAnnotations(new[] { annotation }.Concat(GetAnnotations())); + var annotations = new SortedDictionary(); + + if (Annotations is not null) + { + foreach (var annotation in Annotations.Values) + { + annotations[annotation.Name] = annotation; + } + } + + annotations[name] = new Annotation(name, value); + + return WithAnnotations(annotations); } /// @@ -128,7 +161,7 @@ public virtual TableExpressionBase AddAnnotation(string name, object? value) /// /// The annotations to be applied. /// The new expression with given annotations. - protected abstract TableExpressionBase CreateWithAnnotations(IEnumerable annotations); + protected abstract TableExpressionBase WithAnnotations(IReadOnlyDictionary annotations); /// /// Gets the annotation with the given name, returning if it does not exist. @@ -138,17 +171,13 @@ public virtual TableExpressionBase AddAnnotation(string name, object? value) /// The existing annotation if an annotation with the specified name already exists. Otherwise, . /// public virtual IAnnotation? FindAnnotation(string name) - => _annotations == null - ? null - : _annotations.TryGetValue(name, out var annotation) - ? annotation - : null; + => Annotations?.GetValueOrDefault(name); /// /// Gets all annotations on the current object. /// public virtual IEnumerable GetAnnotations() - => _annotations?.Values ?? Enumerable.Empty(); + => Annotations?.Values ?? Enumerable.Empty(); /// /// This is an internal API that supports the Entity Framework Core infrastructure and not subject to @@ -158,5 +187,5 @@ public virtual IEnumerable GetAnnotations() /// [EntityFrameworkInternal] public virtual string GetRequiredAlias() - => Alias ?? throw new InvalidOperationException($"No alias is defined on table: {ExpressionPrinter.Print(this)}"); + => Alias ?? throw new InvalidOperationException(RelationalStrings.NoAliasOnTable(ExpressionPrinter.Print(this))); } diff --git a/src/EFCore.Relational/Query/SqlExpressions/TableValuedFunctionExpression.cs b/src/EFCore.Relational/Query/SqlExpressions/TableValuedFunctionExpression.cs index 0ef81909162..7d5a2efc1fa 100644 --- a/src/EFCore.Relational/Query/SqlExpressions/TableValuedFunctionExpression.cs +++ b/src/EFCore.Relational/Query/SqlExpressions/TableValuedFunctionExpression.cs @@ -40,13 +40,11 @@ public TableValuedFunctionExpression(string alias, IStoreFunction storeFunction, /// An alias for the table. /// The name of the function. /// The arguments of the function. - /// A collection of annotations associated with this expression. public TableValuedFunctionExpression( string alias, string name, - IReadOnlyList arguments, - IEnumerable? annotations = null) - : this(alias, name, schema: null, builtIn: true, arguments, annotations) + IReadOnlyList arguments) + : this(alias, name, schema: null, builtIn: true, arguments) { } @@ -65,7 +63,7 @@ protected TableValuedFunctionExpression( string? schema, bool builtIn, IReadOnlyList arguments, - IEnumerable? annotations = null) + IReadOnlyDictionary? annotations = null) : base(alias, annotations) { Name = name; @@ -77,12 +75,8 @@ protected TableValuedFunctionExpression( /// /// The alias assigned to this table source. /// - [NotNull] - public override string? Alias - { - get => base.Alias!; - internal set => base.Alias = value; - } + public override string Alias + => base.Alias!; /// /// The store function. @@ -117,7 +111,7 @@ public override string? Alias protected override Expression VisitChildren(ExpressionVisitor visitor) => visitor.VisitAndConvert(Arguments) is var visitedArguments && visitedArguments == Arguments ? this - : new TableValuedFunctionExpression(Alias, Name, Schema, IsBuiltIn, visitedArguments, GetAnnotations()); + : new TableValuedFunctionExpression(Alias, Name, Schema, IsBuiltIn, visitedArguments, Annotations); /// /// Creates a new expression that is like this one, but using the supplied children. If all of the children are the same, it will @@ -127,7 +121,7 @@ protected override Expression VisitChildren(ExpressionVisitor visitor) /// This expression if no children changed, or an expression with the updated children. public virtual TableValuedFunctionExpression Update(IReadOnlyList arguments) => !arguments.SequenceEqual(Arguments) - ? new TableValuedFunctionExpression(Alias, Name, Schema, IsBuiltIn, arguments, GetAnnotations()) + ? new TableValuedFunctionExpression(Alias, Name, Schema, IsBuiltIn, arguments, Annotations) : this; /// @@ -140,7 +134,7 @@ public override TableExpressionBase Clone(string? alias, ExpressionVisitor cloni } var newTableValuedFunctionExpression = StoreFunction is null - ? new TableValuedFunctionExpression(alias!, Name, newArguments) + ? new TableValuedFunctionExpression(alias!, Name, Schema, IsBuiltIn, newArguments) : new TableValuedFunctionExpression(alias!, StoreFunction, newArguments); foreach (var annotation in GetAnnotations()) @@ -152,8 +146,12 @@ public override TableExpressionBase Clone(string? alias, ExpressionVisitor cloni } /// - protected override TableExpressionBase CreateWithAnnotations(IEnumerable annotations) - => new TableValuedFunctionExpression(Alias, Name, Schema, IsBuiltIn, Arguments, annotations); + protected override TableValuedFunctionExpression WithAnnotations(IReadOnlyDictionary annotations) + => new(Alias, Name, Schema, IsBuiltIn, Arguments, annotations); + + /// + public override TableValuedFunctionExpression WithAlias(string newAlias) + => new(newAlias, Name, Schema, IsBuiltIn, Arguments, Annotations); /// protected override void Print(ExpressionPrinter expressionPrinter) diff --git a/src/EFCore.Relational/Query/SqlExpressions/UnionExpression.cs b/src/EFCore.Relational/Query/SqlExpressions/UnionExpression.cs index 879fb73f8cd..7e49663ca00 100644 --- a/src/EFCore.Relational/Query/SqlExpressions/UnionExpression.cs +++ b/src/EFCore.Relational/Query/SqlExpressions/UnionExpression.cs @@ -35,7 +35,7 @@ private UnionExpression( SelectExpression source1, SelectExpression source2, bool distinct, - IEnumerable? annotations) + IReadOnlyDictionary? annotations) : base(alias, source1, source2, distinct, annotations) { } @@ -58,12 +58,12 @@ protected override Expression VisitChildren(ExpressionVisitor visitor) /// This expression if no children changed, or an expression with the updated children. public override UnionExpression Update(SelectExpression source1, SelectExpression source2) => source1 != Source1 || source2 != Source2 - ? new UnionExpression(Alias, source1, source2, IsDistinct, GetAnnotations()) + ? new UnionExpression(Alias, source1, source2, IsDistinct, Annotations) : this; /// - protected override TableExpressionBase CreateWithAnnotations(IEnumerable annotations) - => new UnionExpression(Alias, Source1, Source2, IsDistinct, annotations); + protected override UnionExpression WithAnnotations(IReadOnlyDictionary annotations) + => new(Alias, Source1, Source2, IsDistinct, annotations); /// public override TableExpressionBase Clone(string? alias, ExpressionVisitor cloningExpressionVisitor) @@ -73,6 +73,10 @@ public override TableExpressionBase Clone(string? alias, ExpressionVisitor cloni (SelectExpression)cloningExpressionVisitor.Visit(Source2), IsDistinct); + /// + public override UnionExpression WithAlias(string newAlias) + => new(newAlias, Source1, Source2, IsDistinct); + /// protected override void Print(ExpressionPrinter expressionPrinter) { diff --git a/src/EFCore.Relational/Query/SqlExpressions/UpdateExpression.cs b/src/EFCore.Relational/Query/SqlExpressions/UpdateExpression.cs index 1d276dcb8e9..edcb109e098 100644 --- a/src/EFCore.Relational/Query/SqlExpressions/UpdateExpression.cs +++ b/src/EFCore.Relational/Query/SqlExpressions/UpdateExpression.cs @@ -86,7 +86,7 @@ protected override Expression VisitChildren(ExpressionVisitor visitor) var newValue = (SqlExpression)visitor.Visit(columnValueSetter.Value); if (columnValueSetters != null) { - columnValueSetters.Add(new ColumnValueSetter(columnValueSetter.Column, newValue)); + columnValueSetters.Add(columnValueSetter with { Value = newValue }); } else if (!ReferenceEquals(newValue, columnValueSetter.Value)) { @@ -96,14 +96,15 @@ protected override Expression VisitChildren(ExpressionVisitor visitor) columnValueSetters.Add(ColumnValueSetters[j]); } - columnValueSetters.Add(new ColumnValueSetter(columnValueSetter.Column, newValue)); + columnValueSetters.Add(columnValueSetter with { Value = newValue }); } } - return selectExpression != SelectExpression - || columnValueSetters != null - ? new UpdateExpression(Table, selectExpression, columnValueSetters ?? ColumnValueSetters) - : this; + var table = (TableExpression)visitor.Visit(Table); + + return selectExpression == SelectExpression && table == Table && columnValueSetters is null + ? this + : new UpdateExpression(Table, selectExpression, columnValueSetters ?? ColumnValueSetters); } /// diff --git a/src/EFCore.Relational/Query/SqlExpressions/ValuesExpression.cs b/src/EFCore.Relational/Query/SqlExpressions/ValuesExpression.cs index 6110b0d4ae0..86d44334360 100644 --- a/src/EFCore.Relational/Query/SqlExpressions/ValuesExpression.cs +++ b/src/EFCore.Relational/Query/SqlExpressions/ValuesExpression.cs @@ -48,15 +48,26 @@ public ValuesExpression( ColumnNames = columnNames; } + private ValuesExpression( + string? alias, + IReadOnlyList rowValues, + IReadOnlyList columnNames, + IReadOnlyDictionary? annotations) + : base(alias, annotations) + { + Check.DebugAssert( + rowValues.All(rv => rv.Values.Count == columnNames.Count), + "All row values must have a value count matching the number of column names"); + + RowValues = rowValues; + ColumnNames = columnNames; + } + /// /// The alias assigned to this table source. /// - [NotNull] - public override string? Alias - { - get => base.Alias!; - internal set => base.Alias = value; - } + public override string Alias + => base.Alias!; /// protected override Expression VisitChildren(ExpressionVisitor visitor) @@ -75,8 +86,12 @@ public virtual ValuesExpression Update(IReadOnlyList rowValu : new ValuesExpression(Alias, rowValues, ColumnNames); /// - protected override TableExpressionBase CreateWithAnnotations(IEnumerable annotations) - => new ValuesExpression(Alias, RowValues, ColumnNames, annotations); + protected override ValuesExpression WithAnnotations(IReadOnlyDictionary annotations) + => new(Alias, RowValues, ColumnNames, annotations); + + /// + public override ValuesExpression WithAlias(string newAlias) + => new(newAlias, RowValues, ColumnNames, Annotations); /// public override TableExpressionBase Clone(string? alias, ExpressionVisitor cloningExpressionVisitor) @@ -88,7 +103,7 @@ public override TableExpressionBase Clone(string? alias, ExpressionVisitor cloni newRowValues[i] = (RowValueExpression)cloningExpressionVisitor.Visit(RowValues[i]); } - return new ValuesExpression(alias, newRowValues, ColumnNames, GetAnnotations()); + return new ValuesExpression(alias, newRowValues, ColumnNames, Annotations); } /// diff --git a/src/EFCore.Relational/Query/SqlNullabilityProcessor.cs b/src/EFCore.Relational/Query/SqlNullabilityProcessor.cs index f3330521147..838a24884ad 100644 --- a/src/EFCore.Relational/Query/SqlNullabilityProcessor.cs +++ b/src/EFCore.Relational/Query/SqlNullabilityProcessor.cs @@ -77,7 +77,7 @@ public virtual Expression Process( var result = queryExpression switch { SelectExpression selectExpression => (Expression)Visit(selectExpression), - DeleteExpression deleteExpression => deleteExpression.Update(Visit(deleteExpression.SelectExpression)), + DeleteExpression deleteExpression => deleteExpression.Update(deleteExpression.Table, Visit(deleteExpression.SelectExpression)), UpdateExpression updateExpression => VisitUpdate(updateExpression), _ => throw new InvalidOperationException(), }; diff --git a/src/EFCore.Relational/Query/SqlTreePruner.cs b/src/EFCore.Relational/Query/SqlTreePruner.cs index 0b0d4810bcd..1b6ffbbc54a 100644 --- a/src/EFCore.Relational/Query/SqlTreePruner.cs +++ b/src/EFCore.Relational/Query/SqlTreePruner.cs @@ -74,7 +74,7 @@ protected override Expression VisitExtension(Expression node) Visit(relationalSplitCollectionShaperExpression.InnerShaper)); case DeleteExpression deleteExpression: - return deleteExpression.Update(deleteExpression.SelectExpression.PruneToplevel(this)); + return deleteExpression.Update(deleteExpression.Table, deleteExpression.SelectExpression.PruneToplevel(this)); case UpdateExpression updateExpression: // Note that we must visit the setters before we visit the select, since the setters can reference tables inside it. diff --git a/src/EFCore.SqlServer/Query/Internal/SearchConditionConvertingExpressionVisitor.cs b/src/EFCore.SqlServer/Query/Internal/SearchConditionConvertingExpressionVisitor.cs index 61bb2c69285..217600dfbe5 100644 --- a/src/EFCore.SqlServer/Query/Internal/SearchConditionConvertingExpressionVisitor.cs +++ b/src/EFCore.SqlServer/Query/Internal/SearchConditionConvertingExpressionVisitor.cs @@ -167,7 +167,7 @@ protected override Expression VisitColumn(ColumnExpression columnExpression) /// doing so can result in application failures when updating to a new Entity Framework Core release. /// protected override Expression VisitDelete(DeleteExpression deleteExpression) - => deleteExpression.Update((SelectExpression)Visit(deleteExpression.SelectExpression)); + => deleteExpression.Update(deleteExpression.Table, (SelectExpression)Visit(deleteExpression.SelectExpression)); /// /// This is an internal API that supports the Entity Framework Core infrastructure and not subject to diff --git a/src/EFCore.SqlServer/Query/Internal/SqlServerOpenJsonExpression.cs b/src/EFCore.SqlServer/Query/Internal/SqlServerOpenJsonExpression.cs index a72e17e7bc3..c1bb4bacf62 100644 --- a/src/EFCore.SqlServer/Query/Internal/SqlServerOpenJsonExpression.cs +++ b/src/EFCore.SqlServer/Query/Internal/SqlServerOpenJsonExpression.cs @@ -168,6 +168,10 @@ public override TableExpressionBase Clone(string? alias, ExpressionVisitor cloni return clone; } + /// + public override SqlServerOpenJsonExpression WithAlias(string newAlias) + => new(newAlias, JsonExpression, Path, ColumnInfos); + /// /// This is an internal API that supports the Entity Framework Core infrastructure and not subject to /// the same compatibility standards as public APIs. It may be changed or removed without notice in diff --git a/src/EFCore.SqlServer/Query/Internal/SqlServerQuerySqlGenerator.cs b/src/EFCore.SqlServer/Query/Internal/SqlServerQuerySqlGenerator.cs index c4ca74af05c..62594e04ca4 100644 --- a/src/EFCore.SqlServer/Query/Internal/SqlServerQuerySqlGenerator.cs +++ b/src/EFCore.SqlServer/Query/Internal/SqlServerQuerySqlGenerator.cs @@ -63,11 +63,14 @@ protected override Expression VisitDelete(DeleteExpression deleteExpression) { var selectExpression = deleteExpression.SelectExpression; - if (selectExpression.Offset == null - && selectExpression.Having == null - && selectExpression.Orderings.Count == 0 - && selectExpression.GroupBy.Count == 0 - && selectExpression.Projection.Count == 0) + if (selectExpression is + { + GroupBy: [], + Having: null, + Projection: [], + Orderings: [], + Offset: null + }) { Sql.Append("DELETE "); GenerateTop(selectExpression); @@ -122,11 +125,14 @@ protected override Expression VisitUpdate(UpdateExpression updateExpression) { var selectExpression = updateExpression.SelectExpression; - if (selectExpression.Offset == null - && selectExpression.Having == null - && selectExpression.Orderings.Count == 0 - && selectExpression.GroupBy.Count == 0 - && selectExpression.Projection.Count == 0) + if (selectExpression is + { + GroupBy: [], + Having: null, + Projection: [], + Orderings: [], + Offset: null + }) { Sql.Append("UPDATE "); GenerateTop(selectExpression); diff --git a/src/EFCore.Sqlite.Core/Query/SqlExpressions/Internal/JsonEachExpression.cs b/src/EFCore.Sqlite.Core/Query/SqlExpressions/Internal/JsonEachExpression.cs index d47b1e8d314..741e85bdefc 100644 --- a/src/EFCore.Sqlite.Core/Query/SqlExpressions/Internal/JsonEachExpression.cs +++ b/src/EFCore.Sqlite.Core/Query/SqlExpressions/Internal/JsonEachExpression.cs @@ -142,6 +142,10 @@ public override TableExpressionBase Clone(string? alias, ExpressionVisitor cloni return clone; } + /// + public override JsonEachExpression WithAlias(string newAlias) + => new(newAlias, JsonExpression, Path); + /// /// 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