Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Clean up and open table cloning logic #32665

Merged
merged 1 commit into from
Jan 2, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 0 additions & 3 deletions src/EFCore.Relational/Properties/RelationalStrings.resx
Original file line number Diff line number Diff line change
Expand Up @@ -1134,9 +1134,6 @@
<data name="SubqueryOverComplexTypesNotSupported" xml:space="preserve">
<value>The query requires a subquery over complex type '{complexType}'. Subqueries over complex types are currently unsupported.</value>
</data>
<data name="TableExpressionBaseWithoutCloningLogic" xml:space="preserve">
<value>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}'.</value>
</data>
<data name="TableNotMappedEntityType" xml:space="preserve">
<value>The entity type '{entityType}' is not mapped to the store object '{table}'.</value>
</data>
Expand Down
14 changes: 14 additions & 0 deletions src/EFCore.Relational/Query/Internal/TpcTablesExpression.cs
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,20 @@ protected override Expression VisitChildren(ExpressionVisitor visitor)
protected override TableExpressionBase CreateWithAnnotations(IEnumerable<IAnnotation> annotations)
=> new TpcTablesExpression(Alias, EntityType, SelectExpressions, annotations);

/// <inheritdoc />
public override TableExpressionBase Clone(ExpressionVisitor cloningExpressionVisitor)
{
// Deep clone
var subSelectExpressions = SelectExpressions.Select(cloningExpressionVisitor.Visit).ToList<SelectExpression>();
var newTpcTable = new TpcTablesExpression(Alias, EntityType, subSelectExpressions);
foreach (var annotation in GetAnnotations())
{
newTpcTable.AddAnnotation(annotation.Name, annotation.Value);
}

return newTpcTable;
}

/// <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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ namespace Microsoft.EntityFrameworkCore.Query.SqlExpressions;
/// not used in application code.
/// </para>
/// </summary>
public class FromSqlExpression : TableExpressionBase, ITableBasedExpression, IClonableTableExpressionBase
public class FromSqlExpression : TableExpressionBase, ITableBasedExpression
{
/// <summary>
/// Creates a new instance of the <see cref="FromSqlExpression" /> class.
Expand Down Expand Up @@ -106,7 +106,7 @@ protected override Expression VisitChildren(ExpressionVisitor visitor)
=> this;

/// <inheritdoc />
public virtual TableExpressionBase Clone()
public override TableExpressionBase Clone(ExpressionVisitor cloningVisitor)
=> new FromSqlExpression(Alias, Table, Sql, Arguments, GetAnnotations());

/// <inheritdoc />
Expand Down

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,12 @@ protected JoinExpressionBase(TableExpressionBase table, bool prunable, IEnumerab
/// <returns>This expression if no children changed, or an expression with the updated children.</returns>
public abstract JoinExpressionBase Update(TableExpressionBase table);

/// <inheritdoc />
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);

/// <inheritdoc />
public override bool Equals(object? obj)
=> obj != null
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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<ProjectionMember, Expression>(selectExpression._projectionMapping.Count);
foreach (var (projectionMember, value) in selectExpression._projectionMapping)
{
newProjectionMappings[projectionMember] = Visit(value);
}

var newProjections = selectExpression._projection.Select(Visit).ToList<ProjectionExpression>();

var newTables = selectExpression._tables.Select(Visit).ToList<TableExpressionBase>();
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<SqlExpression>();
var havingExpression = (SqlExpression?)Visit(selectExpression.Having);
var newOrderings = selectExpression._orderings.Select(Visit).ToList<OrderingExpression>();
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<SelectExpression>();
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
Expand Down
74 changes: 74 additions & 0 deletions src/EFCore.Relational/Query/SqlExpressions/SelectExpression.cs
Original file line number Diff line number Diff line change
Expand Up @@ -4341,6 +4341,80 @@ public SelectExpression Clone()
return (SelectExpression)_cloningExpressionVisitor.Visit(this);
}

/// <summary>
/// Creates a new object that is a copy of the current instance.
/// </summary>
/// <param name="cloningExpressionVisitor">The cloning expression for further visitation of nested nodes.</param>
/// <returns>A new object that is a copy of this instance.</returns>
public override TableExpressionBase Clone(ExpressionVisitor cloningExpressionVisitor)
{
var newProjectionMappings = new Dictionary<ProjectionMember, Expression>(_projectionMapping.Count);
foreach (var (projectionMember, value) in _projectionMapping)
{
newProjectionMappings[projectionMember] = cloningExpressionVisitor.Visit(value);
}

var newProjections = _projection.Select(cloningExpressionVisitor.Visit).ToList<ProjectionExpression>();

var newTables = _tables.Select(cloningExpressionVisitor.Visit).ToList<TableExpressionBase>();
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<SqlExpression>();
var havingExpression = (SqlExpression?)cloningExpressionVisitor.Visit(Having);
var newOrderings = _orderings.Select(cloningExpressionVisitor.Visit).ToList<OrderingExpression>();
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;
}

/// <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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,12 @@ public override string? Alias
/// <returns>This expression if no children changed, or an expression with the updated children.</returns>
public abstract SetOperationBase Update(SelectExpression source1, SelectExpression source2);

/// <inheritdoc />
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);

/// <inheritdoc />
public override bool Equals(object? obj)
=> obj != null
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 <see href="https://github.com/dotnet/efcore">github.com/dotnet/efcore</see>.
/// </remarks>
public sealed class TableExpression : TableExpressionBase, IClonableTableExpressionBase, ITableBasedExpression
public sealed class TableExpression : TableExpressionBase, ITableBasedExpression
{
internal TableExpression(ITableBase table)
: this(table, annotations: null)
Expand Down Expand Up @@ -76,7 +76,7 @@ protected override void Print(ExpressionPrinter expressionPrinter)
}

/// <inheritdoc />
public TableExpressionBase Clone()
public override TableExpressionBase Clone(ExpressionVisitor cloningExpressionVisitor)
=> CreateWithAnnotations(GetAnnotations());

/// <inheritdoc />
Expand Down
Loading
Loading