From 01a8584e9e02f208bbca596d37ec83198f5d90ad Mon Sep 17 00:00:00 2001 From: Shay Rojansky Date: Sun, 24 Dec 2023 08:54:58 +0100 Subject: [PATCH] Clean up and open table cloning logic Fixes #30982 Fixes #32664 --- .../Properties/RelationalStrings.Designer.cs | 8 -- .../Properties/RelationalStrings.resx | 3 - .../Query/Internal/TpcTablesExpression.cs | 14 ++ .../Query/SqlExpressions/FromSqlExpression.cs | 4 +- .../IClonableTableExpressionBase.cs | 22 --- .../SqlExpressions/JoinExpressionBase.cs | 6 + .../SqlExpressions/SelectExpression.Helper.cs | 132 +----------------- .../Query/SqlExpressions/SelectExpression.cs | 74 ++++++++++ .../Query/SqlExpressions/SetOperationBase.cs | 6 + .../Query/SqlExpressions/TableExpression.cs | 4 +- .../SqlExpressions/TableExpressionBase.cs | 7 + .../TableValuedFunctionExpression.cs | 21 +++ .../Query/SqlExpressions/ValuesExpression.cs | 16 ++- .../Internal/SqlServerOpenJsonExpression.cs | 8 +- .../Internal/JsonEachExpression.cs | 8 +- 15 files changed, 156 insertions(+), 177 deletions(-) delete mode 100644 src/EFCore.Relational/Query/SqlExpressions/IClonableTableExpressionBase.cs diff --git a/src/EFCore.Relational/Properties/RelationalStrings.Designer.cs b/src/EFCore.Relational/Properties/RelationalStrings.Designer.cs index dee56b4839e..648679a1ad5 100644 --- a/src/EFCore.Relational/Properties/RelationalStrings.Designer.cs +++ b/src/EFCore.Relational/Properties/RelationalStrings.Designer.cs @@ -1867,14 +1867,6 @@ public static string SubqueryOverComplexTypesNotSupported(object? complexType) GetString("SubqueryOverComplexTypesNotSupported", nameof(complexType)), complexType); - /// - /// Expression type '{expressionType}' does not implement proper cloning logic. Every expression derived from '{tableExpressionBase}' must implement '{clonableTableExpressionBase}' interface or have it's cloning logic added to the '{cloningExpressionVisitor}' inside '{selectExpression}'. - /// - public static string TableExpressionBaseWithoutCloningLogic(object? expressionType, object? tableExpressionBase, object? clonableTableExpressionBase, object? cloningExpressionVisitor, object? selectExpression) - => string.Format( - GetString("TableExpressionBaseWithoutCloningLogic", nameof(expressionType), nameof(tableExpressionBase), nameof(clonableTableExpressionBase), nameof(cloningExpressionVisitor), nameof(selectExpression)), - expressionType, tableExpressionBase, clonableTableExpressionBase, cloningExpressionVisitor, selectExpression); - /// /// The entity type '{entityType}' is not mapped to the store object '{table}'. /// diff --git a/src/EFCore.Relational/Properties/RelationalStrings.resx b/src/EFCore.Relational/Properties/RelationalStrings.resx index 2ea8a3f06da..f19a7c84675 100644 --- a/src/EFCore.Relational/Properties/RelationalStrings.resx +++ b/src/EFCore.Relational/Properties/RelationalStrings.resx @@ -1134,9 +1134,6 @@ The query requires a subquery over complex type '{complexType}'. Subqueries over complex types are currently unsupported. - - Expression type '{expressionType}' does not implement proper cloning logic. Every expression derived from '{tableExpressionBase}' must implement '{clonableTableExpressionBase}' interface or have it's cloning logic added to the '{cloningExpressionVisitor}' inside '{selectExpression}'. - The entity type '{entityType}' is not mapped to the store object '{table}'. diff --git a/src/EFCore.Relational/Query/Internal/TpcTablesExpression.cs b/src/EFCore.Relational/Query/Internal/TpcTablesExpression.cs index 06f161edce7..f65991e028a 100644 --- a/src/EFCore.Relational/Query/Internal/TpcTablesExpression.cs +++ b/src/EFCore.Relational/Query/Internal/TpcTablesExpression.cs @@ -108,6 +108,20 @@ protected override Expression VisitChildren(ExpressionVisitor visitor) protected override TableExpressionBase CreateWithAnnotations(IEnumerable annotations) => new TpcTablesExpression(Alias, EntityType, SelectExpressions, annotations); + /// + public override TableExpressionBase Clone(ExpressionVisitor cloningExpressionVisitor) + { + // Deep clone + var subSelectExpressions = SelectExpressions.Select(cloningExpressionVisitor.Visit).ToList(); + var newTpcTable = new TpcTablesExpression(Alias, EntityType, subSelectExpressions); + foreach (var annotation in GetAnnotations()) + { + newTpcTable.AddAnnotation(annotation.Name, annotation.Value); + } + + return newTpcTable; + } + /// /// 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/FromSqlExpression.cs b/src/EFCore.Relational/Query/SqlExpressions/FromSqlExpression.cs index b82c4944f42..523216c9893 100644 --- a/src/EFCore.Relational/Query/SqlExpressions/FromSqlExpression.cs +++ b/src/EFCore.Relational/Query/SqlExpressions/FromSqlExpression.cs @@ -14,7 +14,7 @@ namespace Microsoft.EntityFrameworkCore.Query.SqlExpressions; /// not used in application code. /// /// -public class FromSqlExpression : TableExpressionBase, ITableBasedExpression, IClonableTableExpressionBase +public class FromSqlExpression : TableExpressionBase, ITableBasedExpression { /// /// Creates a new instance of the class. @@ -106,7 +106,7 @@ protected override Expression VisitChildren(ExpressionVisitor visitor) => this; /// - public virtual TableExpressionBase Clone() + public override TableExpressionBase Clone(ExpressionVisitor cloningVisitor) => new FromSqlExpression(Alias, Table, Sql, Arguments, GetAnnotations()); /// diff --git a/src/EFCore.Relational/Query/SqlExpressions/IClonableTableExpressionBase.cs b/src/EFCore.Relational/Query/SqlExpressions/IClonableTableExpressionBase.cs deleted file mode 100644 index d3cef2cd979..00000000000 --- a/src/EFCore.Relational/Query/SqlExpressions/IClonableTableExpressionBase.cs +++ /dev/null @@ -1,22 +0,0 @@ -// 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.SqlExpressions; - -/// -/// -/// An interface that represents a table source in a SQL tree which can be cloned. -/// -/// -/// This interface is typically used by database providers (and other extensions). It is generally -/// not used in application code. -/// -/// -public interface IClonableTableExpressionBase -{ - /// - /// Creates a new object that is a copy of the current instance. - /// - /// A new object that is a copy of this instance. - TableExpressionBase Clone(); -} diff --git a/src/EFCore.Relational/Query/SqlExpressions/JoinExpressionBase.cs b/src/EFCore.Relational/Query/SqlExpressions/JoinExpressionBase.cs index ba221e7ed88..d427520215e 100644 --- a/src/EFCore.Relational/Query/SqlExpressions/JoinExpressionBase.cs +++ b/src/EFCore.Relational/Query/SqlExpressions/JoinExpressionBase.cs @@ -46,6 +46,12 @@ protected JoinExpressionBase(TableExpressionBase table, bool prunable, IEnumerab /// This expression if no children changed, or an expression with the updated children. public abstract JoinExpressionBase Update(TableExpressionBase table); + /// + public override TableExpressionBase Clone(ExpressionVisitor cloningExpressionVisitor) + // Joins necessary contain other TableExpressionBase, which will get cloned; this will cause our VisitChildren to create a new + // copy of this JoinExpressionBase by calling Update. + => (TableExpressionBase)VisitChildren(cloningExpressionVisitor); + /// public override bool Equals(object? obj) => obj != null diff --git a/src/EFCore.Relational/Query/SqlExpressions/SelectExpression.Helper.cs b/src/EFCore.Relational/Query/SqlExpressions/SelectExpression.Helper.cs index bfbcae94c6d..70a0e744aed 100644 --- a/src/EFCore.Relational/Query/SqlExpressions/SelectExpression.Helper.cs +++ b/src/EFCore.Relational/Query/SqlExpressions/SelectExpression.Helper.cs @@ -753,136 +753,12 @@ private sealed class CloningExpressionVisitor : ExpressionVisitor { [return: NotNullIfNotNull("expression")] public override Expression? Visit(Expression? expression) - { - switch (expression) + => expression switch { - case SelectExpression selectExpression: - { - var newProjectionMappings = new Dictionary(selectExpression._projectionMapping.Count); - foreach (var (projectionMember, value) in selectExpression._projectionMapping) - { - newProjectionMappings[projectionMember] = Visit(value); - } - - var newProjections = selectExpression._projection.Select(Visit).ToList(); - - var newTables = selectExpression._tables.Select(Visit).ToList(); - var tpcTablesMap = selectExpression._tables.Select(UnwrapJoinExpression).Zip(newTables.Select(UnwrapJoinExpression)) - .Where(e => e.First is TpcTablesExpression) - .ToDictionary(e => (TpcTablesExpression)e.First, e => (TpcTablesExpression)e.Second); - - // Since we are cloning we need to generate new table references - // In other cases (like VisitChildren), we just reuse the same table references and update the SelectExpression inside it. - // We initially assign old SelectExpression in table references and later update it once we construct clone - var newTableReferences = selectExpression._tableReferences - .Select(e => new TableReferenceExpression(selectExpression, e.Alias)).ToList(); - Check.DebugAssert( - newTables.Select(GetAliasFromTableExpressionBase).SequenceEqual(newTableReferences.Select(e => e.Alias)), - "Alias of updated tables must match the old tables."); - - var predicate = (SqlExpression?)Visit(selectExpression.Predicate); - var newGroupBy = selectExpression._groupBy.Select(Visit) - .Where(e => e is not (SqlConstantExpression or SqlParameterExpression)) - .ToList(); - var havingExpression = (SqlExpression?)Visit(selectExpression.Having); - var newOrderings = selectExpression._orderings.Select(Visit).ToList(); - var offset = (SqlExpression?)Visit(selectExpression.Offset); - var limit = (SqlExpression?)Visit(selectExpression.Limit); - - var newSelectExpression = new SelectExpression( - selectExpression.Alias, newProjections, newTables, newTableReferences, newGroupBy, newOrderings, - selectExpression.GetAnnotations()) - { - Predicate = predicate, - Having = havingExpression, - Offset = offset, - Limit = limit, - IsDistinct = selectExpression.IsDistinct, - Tags = selectExpression.Tags, - _usedAliases = selectExpression._usedAliases.ToHashSet(), - _projectionMapping = newProjectionMappings, - _mutable = selectExpression._mutable - }; - - foreach (var kvp in selectExpression._tpcDiscriminatorValues) - { - newSelectExpression._tpcDiscriminatorValues[tpcTablesMap[kvp.Key]] = kvp.Value; - } - - // Since identifiers are ColumnExpression, they are not visited since they don't contain SelectExpression inside it. - newSelectExpression._identifier.AddRange(selectExpression._identifier); - newSelectExpression._childIdentifiers.AddRange(selectExpression._childIdentifiers); - - // Remap tableReferences in new select expression - foreach (var tableReference in newTableReferences) - { - tableReference.UpdateTableReference(selectExpression, newSelectExpression); - } - - // Now that we have SelectExpression, we visit all components and update table references inside columns - newSelectExpression = (SelectExpression)new ColumnExpressionReplacingExpressionVisitor( - selectExpression, newSelectExpression._tableReferences).Visit(newSelectExpression); - - return newSelectExpression; - } - - case TpcTablesExpression tpcTablesExpression: - { - // Deep clone - var subSelectExpressions = tpcTablesExpression.SelectExpressions.Select(Visit).ToList(); - var newTpcTable = new TpcTablesExpression( - tpcTablesExpression.Alias, tpcTablesExpression.EntityType, subSelectExpressions); - foreach (var annotation in tpcTablesExpression.GetAnnotations()) - { - newTpcTable.AddAnnotation(annotation.Name, annotation.Value); - } + TableExpressionBase table => table.Clone(this), - return newTpcTable; - } - - case IClonableTableExpressionBase cloneable: - return cloneable.Clone(); - - case TableValuedFunctionExpression tableValuedFunctionExpression: - { - var newArguments = new SqlExpression[tableValuedFunctionExpression.Arguments.Count]; - for (var i = 0; i < newArguments.Length; i++) - { - newArguments[i] = (SqlExpression)Visit(tableValuedFunctionExpression.Arguments[i]); - } - - var newTableValuedFunctionExpression = tableValuedFunctionExpression.StoreFunction is null - ? new TableValuedFunctionExpression( - tableValuedFunctionExpression.Alias, tableValuedFunctionExpression.Name, newArguments) - : new TableValuedFunctionExpression( - tableValuedFunctionExpression.StoreFunction, newArguments) { Alias = tableValuedFunctionExpression.Alias }; - - foreach (var annotation in tableValuedFunctionExpression.GetAnnotations()) - { - newTableValuedFunctionExpression.AddAnnotation(annotation.Name, annotation.Value); - } - - return newTableValuedFunctionExpression; - } - - // join and set operations are fine, because they contain other TableExpressionBases inside, that will get cloned - // and therefore set expression's Update function will generate a new instance. - case JoinExpressionBase or SetOperationBase: - return base.Visit(expression); - - case TableExpressionBase: - throw new InvalidOperationException( - RelationalStrings.TableExpressionBaseWithoutCloningLogic( - expression.GetType().Name, - nameof(TableExpressionBase), - nameof(IClonableTableExpressionBase), - nameof(CloningExpressionVisitor), - nameof(SelectExpression))); - - default: - return base.Visit(expression); - } - } + _ => base.Visit(expression) + }; } private sealed class ColumnExpressionReplacingExpressionVisitor : ExpressionVisitor diff --git a/src/EFCore.Relational/Query/SqlExpressions/SelectExpression.cs b/src/EFCore.Relational/Query/SqlExpressions/SelectExpression.cs index 40945a0ed81..ca2a09a9b96 100644 --- a/src/EFCore.Relational/Query/SqlExpressions/SelectExpression.cs +++ b/src/EFCore.Relational/Query/SqlExpressions/SelectExpression.cs @@ -4341,6 +4341,80 @@ public SelectExpression Clone() return (SelectExpression)_cloningExpressionVisitor.Visit(this); } + /// + /// Creates a new object that is a copy of the current instance. + /// + /// The cloning expression for further visitation of nested nodes. + /// A new object that is a copy of this instance. + public override TableExpressionBase Clone(ExpressionVisitor cloningExpressionVisitor) + { + var newProjectionMappings = new Dictionary(_projectionMapping.Count); + foreach (var (projectionMember, value) in _projectionMapping) + { + newProjectionMappings[projectionMember] = cloningExpressionVisitor.Visit(value); + } + + var newProjections = _projection.Select(cloningExpressionVisitor.Visit).ToList(); + + var newTables = _tables.Select(cloningExpressionVisitor.Visit).ToList(); + var tpcTablesMap = _tables.Select(UnwrapJoinExpression).Zip(newTables.Select(UnwrapJoinExpression)) + .Where(e => e.First is TpcTablesExpression) + .ToDictionary(e => (TpcTablesExpression)e.First, e => (TpcTablesExpression)e.Second); + + // Since we are cloning we need to generate new table references + // In other cases (like VisitChildren), we just reuse the same table references and update the SelectExpression inside it. + // We initially assign old SelectExpression in table references and later update it once we construct clone + var newTableReferences = _tableReferences.Select(e => new TableReferenceExpression(this, e.Alias)).ToList(); + Check.DebugAssert( + newTables.Select(GetAliasFromTableExpressionBase).SequenceEqual(newTableReferences.Select(e => e.Alias)), + "Alias of updated tables must match the old tables."); + + var predicate = (SqlExpression?)cloningExpressionVisitor.Visit(Predicate); + var newGroupBy = _groupBy.Select(cloningExpressionVisitor.Visit) + .Where(e => e is not (SqlConstantExpression or SqlParameterExpression)) + .ToList(); + var havingExpression = (SqlExpression?)cloningExpressionVisitor.Visit(Having); + var newOrderings = _orderings.Select(cloningExpressionVisitor.Visit).ToList(); + var offset = (SqlExpression?)cloningExpressionVisitor.Visit(Offset); + var limit = (SqlExpression?)cloningExpressionVisitor.Visit(Limit); + + var newSelectExpression = new SelectExpression( + Alias, newProjections, newTables, newTableReferences, newGroupBy, newOrderings, + GetAnnotations()) + { + Predicate = predicate, + Having = havingExpression, + Offset = offset, + Limit = limit, + IsDistinct = IsDistinct, + Tags = Tags, + _usedAliases = _usedAliases.ToHashSet(), + _projectionMapping = newProjectionMappings, + _mutable = _mutable + }; + + foreach (var kvp in _tpcDiscriminatorValues) + { + newSelectExpression._tpcDiscriminatorValues[tpcTablesMap[kvp.Key]] = kvp.Value; + } + + // Since identifiers are ColumnExpression, they are not visited since they don't contain SelectExpression inside it. + newSelectExpression._identifier.AddRange(_identifier); + newSelectExpression._childIdentifiers.AddRange(_childIdentifiers); + + // Remap tableReferences in new select expression + foreach (var tableReference in newTableReferences) + { + tableReference.UpdateTableReference(this, newSelectExpression); + } + + // Now that we have SelectExpression, we visit all components and update table references inside columns + newSelectExpression = (SelectExpression)new ColumnExpressionReplacingExpressionVisitor( + this, newSelectExpression._tableReferences).Visit(newSelectExpression); + + return newSelectExpression; + } + /// /// 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/SetOperationBase.cs b/src/EFCore.Relational/Query/SqlExpressions/SetOperationBase.cs index aa324cf1c97..f59e28c18b4 100644 --- a/src/EFCore.Relational/Query/SqlExpressions/SetOperationBase.cs +++ b/src/EFCore.Relational/Query/SqlExpressions/SetOperationBase.cs @@ -87,6 +87,12 @@ public override string? Alias /// This expression if no children changed, or an expression with the updated children. public abstract SetOperationBase Update(SelectExpression source1, SelectExpression source2); + /// + public override TableExpressionBase Clone(ExpressionVisitor cloningExpressionVisitor) + // Set operations necessary contain other TableExpressionBase, which will get cloned; this will cause our VisitChildren to create a + // new copy of this SetOperationBase by calling Update. + => (TableExpressionBase)VisitChildren(cloningExpressionVisitor); + /// public override bool Equals(object? obj) => obj != null diff --git a/src/EFCore.Relational/Query/SqlExpressions/TableExpression.cs b/src/EFCore.Relational/Query/SqlExpressions/TableExpression.cs index d8b47cc6fba..f65b0eeeb0b 100644 --- a/src/EFCore.Relational/Query/SqlExpressions/TableExpression.cs +++ b/src/EFCore.Relational/Query/SqlExpressions/TableExpression.cs @@ -13,7 +13,7 @@ namespace Microsoft.EntityFrameworkCore.Query.SqlExpressions; /// application or database provider code. If this is a problem for your application or provider, then please file /// an issue at github.com/dotnet/efcore. /// -public sealed class TableExpression : TableExpressionBase, IClonableTableExpressionBase, ITableBasedExpression +public sealed class TableExpression : TableExpressionBase, ITableBasedExpression { internal TableExpression(ITableBase table) : this(table, annotations: null) @@ -76,7 +76,7 @@ protected override void Print(ExpressionPrinter expressionPrinter) } /// - public TableExpressionBase Clone() + public override TableExpressionBase Clone(ExpressionVisitor cloningExpressionVisitor) => CreateWithAnnotations(GetAnnotations()); /// diff --git a/src/EFCore.Relational/Query/SqlExpressions/TableExpressionBase.cs b/src/EFCore.Relational/Query/SqlExpressions/TableExpressionBase.cs index 63a0ca7f5a6..73f85eead62 100644 --- a/src/EFCore.Relational/Query/SqlExpressions/TableExpressionBase.cs +++ b/src/EFCore.Relational/Query/SqlExpressions/TableExpressionBase.cs @@ -55,6 +55,13 @@ public override Type Type public sealed override ExpressionType NodeType => ExpressionType.Extension; + /// + /// Creates a new object that is a copy of the current instance. + /// + /// The cloning expression for further visitation of nested nodes. + /// A new object that is a copy of this instance. + public abstract TableExpressionBase Clone(ExpressionVisitor cloningExpressionVisitor); + /// /// Creates a printable string representation of the given expression using . /// diff --git a/src/EFCore.Relational/Query/SqlExpressions/TableValuedFunctionExpression.cs b/src/EFCore.Relational/Query/SqlExpressions/TableValuedFunctionExpression.cs index 85e38459a57..ad58e17ce57 100644 --- a/src/EFCore.Relational/Query/SqlExpressions/TableValuedFunctionExpression.cs +++ b/src/EFCore.Relational/Query/SqlExpressions/TableValuedFunctionExpression.cs @@ -143,6 +143,27 @@ public virtual TableValuedFunctionExpression Update(IReadOnlyList ? new TableValuedFunctionExpression(Alias, Name, Schema, IsBuiltIn, arguments, GetAnnotations()) : this; + /// + public override TableExpressionBase Clone(ExpressionVisitor cloningExpressionVisitor) + { + var newArguments = new SqlExpression[Arguments.Count]; + for (var i = 0; i < newArguments.Length; i++) + { + newArguments[i] = (SqlExpression)cloningExpressionVisitor.Visit(Arguments[i]); + } + + var newTableValuedFunctionExpression = StoreFunction is null + ? new TableValuedFunctionExpression(Alias, Name, newArguments) + : new TableValuedFunctionExpression(StoreFunction, newArguments) { Alias = Alias }; + + foreach (var annotation in GetAnnotations()) + { + newTableValuedFunctionExpression.AddAnnotation(annotation.Name, annotation.Value); + } + + return newTableValuedFunctionExpression; + } + /// protected override TableExpressionBase CreateWithAnnotations(IEnumerable annotations) => new TableValuedFunctionExpression(Alias, Name, Schema, IsBuiltIn, Arguments, annotations); diff --git a/src/EFCore.Relational/Query/SqlExpressions/ValuesExpression.cs b/src/EFCore.Relational/Query/SqlExpressions/ValuesExpression.cs index 366bb3de8c4..d070e68d7cf 100644 --- a/src/EFCore.Relational/Query/SqlExpressions/ValuesExpression.cs +++ b/src/EFCore.Relational/Query/SqlExpressions/ValuesExpression.cs @@ -14,7 +14,7 @@ namespace Microsoft.EntityFrameworkCore.Query.SqlExpressions; /// not used in application code. /// /// -public class ValuesExpression : TableExpressionBase, IClonableTableExpressionBase +public class ValuesExpression : TableExpressionBase { /// /// The row values for this table. @@ -78,10 +78,18 @@ public virtual ValuesExpression Update(IReadOnlyList rowValu protected override TableExpressionBase CreateWithAnnotations(IEnumerable annotations) => new ValuesExpression(Alias, RowValues, ColumnNames, annotations); - // TODO: Deep clone, see #30982 /// - public virtual TableExpressionBase Clone() - => CreateWithAnnotations(GetAnnotations()); + public override TableExpressionBase Clone(ExpressionVisitor cloningExpressionVisitor) + { + var newRowValues = new RowValueExpression[RowValues.Count]; + + for (var i = 0; i < newRowValues.Length; i++) + { + newRowValues[i] = (RowValueExpression)cloningExpressionVisitor.Visit(RowValues[i]); + } + + return new ValuesExpression(Alias, newRowValues, ColumnNames, GetAnnotations()); + } /// protected override void Print(ExpressionPrinter expressionPrinter) diff --git a/src/EFCore.SqlServer/Query/Internal/SqlServerOpenJsonExpression.cs b/src/EFCore.SqlServer/Query/Internal/SqlServerOpenJsonExpression.cs index 21ff76e15fb..be93c8485cd 100644 --- a/src/EFCore.SqlServer/Query/Internal/SqlServerOpenJsonExpression.cs +++ b/src/EFCore.SqlServer/Query/Internal/SqlServerOpenJsonExpression.cs @@ -20,7 +20,7 @@ namespace Microsoft.EntityFrameworkCore.SqlServer.Query.Internal; /// doing so can result in application failures when updating to a new Entity Framework Core release. /// /// -public class SqlServerOpenJsonExpression : TableValuedFunctionExpression, IClonableTableExpressionBase +public class SqlServerOpenJsonExpression : TableValuedFunctionExpression { /// /// This is an internal API that supports the Entity Framework Core infrastructure and not subject to @@ -155,10 +155,10 @@ public virtual SqlServerOpenJsonExpression Update( /// 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. /// - // TODO: Deep clone, see #30982 - public virtual TableExpressionBase Clone() + public override TableExpressionBase Clone(ExpressionVisitor cloningExpressionVisitor) { - var clone = new SqlServerOpenJsonExpression(Alias, JsonExpression, Path, ColumnInfos); + var newJsonExpression = (SqlExpression)cloningExpressionVisitor.Visit(JsonExpression); + var clone = new SqlServerOpenJsonExpression(Alias, newJsonExpression, Path, ColumnInfos); foreach (var annotation in GetAnnotations()) { diff --git a/src/EFCore.Sqlite.Core/Query/SqlExpressions/Internal/JsonEachExpression.cs b/src/EFCore.Sqlite.Core/Query/SqlExpressions/Internal/JsonEachExpression.cs index 0a45c8c784d..4e3b6307faa 100644 --- a/src/EFCore.Sqlite.Core/Query/SqlExpressions/Internal/JsonEachExpression.cs +++ b/src/EFCore.Sqlite.Core/Query/SqlExpressions/Internal/JsonEachExpression.cs @@ -19,7 +19,7 @@ namespace Microsoft.EntityFrameworkCore.Sqlite.Query.SqlExpressions.Internal; /// doing so can result in application failures when updating to a new Entity Framework Core release. /// /// -public class JsonEachExpression : TableValuedFunctionExpression, IClonableTableExpressionBase +public class JsonEachExpression : TableValuedFunctionExpression { /// /// This is an internal API that supports the Entity Framework Core infrastructure and not subject to @@ -129,10 +129,10 @@ public virtual JsonEachExpression Update( /// 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. /// - // TODO: Deep clone, see #30982 - public virtual TableExpressionBase Clone() + public override TableExpressionBase Clone(ExpressionVisitor cloningExpressionVisitor) { - var clone = new JsonEachExpression(Alias, JsonExpression, Path); + var newJsonExpression = (SqlExpression)cloningExpressionVisitor.Visit(JsonExpression); + var clone = new JsonEachExpression(Alias, newJsonExpression, Path); foreach (var annotation in GetAnnotations()) {