Skip to content
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
16 changes: 15 additions & 1 deletion src/EFCore.Relational/Properties/RelationalStrings.Designer.cs

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

6 changes: 6 additions & 0 deletions src/EFCore.Relational/Properties/RelationalStrings.resx
Original file line number Diff line number Diff line change
Expand Up @@ -136,6 +136,9 @@
<data name="CannotProjectNullableComplexType" xml:space="preserve">
<value>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.</value>
</data>
<data name="CannotSetAliasOnJoin" xml:space="preserve">
<value>Join expressions have no aliases; set the alias on the enclosed table expression.</value>
</data>
<data name="CannotTranslateNonConstantNewArrayExpression" xml:space="preserve">
<value>The query contained a new array expression containing non-constant elements, which could not be translated: '{newArrayExpression}'.</value>
</data>
Expand Down Expand Up @@ -927,6 +930,9 @@
<data name="NoActiveTransaction" xml:space="preserve">
<value>The connection does not have any active transactions.</value>
</data>
<data name="NoAliasOnTable" xml:space="preserve">
<value>No alias is defined on table: '{table}'.</value>
</data>
<data name="NoDbCommand" xml:space="preserve">
<value>Cannot create a DbCommand for a non-relational query.</value>
</data>
Expand Down
20 changes: 10 additions & 10 deletions src/EFCore.Relational/Query/Internal/TpcTablesExpression.cs
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ private TpcTablesExpression(
string? alias,
IEntityType entityType,
IReadOnlyList<SelectExpression> subSelectExpressions,
IEnumerable<IAnnotation>? annotations)
IReadOnlyDictionary<string, IAnnotation>? annotations)
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@maumar the changes around annotations are to stop duplicating/copying them every time we copy/clone the table expression node; since they're immutable we just reuse the existing dictionary, as is standard with expressions.

: base(alias, annotations)
{
EntityType = entityType;
Expand All @@ -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.
/// </summary>
[NotNull]
public override string? Alias
{
get => base.Alias!;
internal set => base.Alias = value;
}
public override string Alias
=> base.Alias!;

/// <summary>
/// This is an internal API that supports the Entity Framework Core infrastructure and not subject to
Expand Down Expand Up @@ -86,7 +82,7 @@ public TpcTablesExpression Prune(IReadOnlyList<string> 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);
}

/// <summary>
Expand All @@ -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.
/// </summary>
protected override TableExpressionBase CreateWithAnnotations(IEnumerable<IAnnotation> annotations)
=> new TpcTablesExpression(Alias, EntityType, SelectExpressions, annotations);
protected override TpcTablesExpression WithAnnotations(IReadOnlyDictionary<string, IAnnotation> annotations)
=> new(Alias, EntityType, SelectExpressions, annotations);

/// <inheritdoc />
public override TpcTablesExpression WithAlias(string newAlias)
=> new(newAlias, EntityType, SelectExpressions, Annotations);

/// <inheritdoc />
public override TableExpressionBase Clone(string? alias, ExpressionVisitor cloningExpressionVisitor)
Expand Down
34 changes: 20 additions & 14 deletions src/EFCore.Relational/Query/QuerySqlGenerator.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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
Expand Down
15 changes: 1 addition & 14 deletions src/EFCore.Relational/Query/SqlAliasManager.cs
Original file line number Diff line number Diff line change
Expand Up @@ -168,8 +168,6 @@ protected override Expression VisitExtension(Expression node)

private sealed class TableAliasRewriter(IReadOnlyDictionary<string, string> aliasRewritingMap) : ExpressionVisitor
{
private readonly HashSet<TableExpressionBase> _visitedTables = new(ReferenceEqualityComparer.Instance);

internal static Expression Rewrite(Expression expression, IReadOnlyDictionary<string, string> aliasRewritingMap)
=> new TableAliasRewriter(aliasRewritingMap).Visit(expression);

Expand All @@ -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);
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was the one point left actually mutating aliases - an immutable copy of the table is now produced instead.

}

return base.VisitExtension(table);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ public CrossApplyExpression(TableExpressionBase table)
{
}

private CrossApplyExpression(TableExpressionBase table, IEnumerable<IAnnotation>? annotations)
private CrossApplyExpression(TableExpressionBase table, IReadOnlyDictionary<string, IAnnotation>? annotations)
: base(table, prunable: false, annotations)
{
}
Expand All @@ -40,12 +40,12 @@ protected override Expression VisitChildren(ExpressionVisitor visitor)
/// <returns>This expression if no children changed, or an expression with the updated children.</returns>
public override CrossApplyExpression Update(TableExpressionBase table)
=> table != Table
? new CrossApplyExpression(table, GetAnnotations())
? new CrossApplyExpression(table, Annotations)
: this;

/// <inheritdoc />
protected override TableExpressionBase CreateWithAnnotations(IEnumerable<IAnnotation> annotations)
=> new CrossApplyExpression(Table, GetAnnotations());
protected override CrossApplyExpression WithAnnotations(IReadOnlyDictionary<string, IAnnotation> annotations)
=> new(Table, annotations);

/// <inheritdoc />
protected override void Print(ExpressionPrinter expressionPrinter)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ public CrossJoinExpression(TableExpressionBase table)
{
}

private CrossJoinExpression(TableExpressionBase table, IEnumerable<IAnnotation>? annotations)
private CrossJoinExpression(TableExpressionBase table, IReadOnlyDictionary<string, IAnnotation>? annotations)
: base(table, prunable: false, annotations)
{
}
Expand All @@ -40,12 +40,12 @@ protected override Expression VisitChildren(ExpressionVisitor visitor)
/// <returns>This expression if no children changed, or an expression with the updated children.</returns>
public override CrossJoinExpression Update(TableExpressionBase table)
=> table != Table
? new CrossJoinExpression(table, GetAnnotations())
? new CrossJoinExpression(table, Annotations)
: this;

/// <inheritdoc />
protected override TableExpressionBase CreateWithAnnotations(IEnumerable<IAnnotation> annotations)
=> new CrossJoinExpression(Table, annotations);
protected override CrossJoinExpression WithAnnotations(IReadOnlyDictionary<string, IAnnotation> annotations)
=> new(Table, Annotations);

/// <inheritdoc />
protected override void Print(ExpressionPrinter expressionPrinter)
Expand Down
13 changes: 7 additions & 6 deletions src/EFCore.Relational/Query/SqlExpressions/DeleteExpression.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}

/// <summary>
/// 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.
/// </summary>
/// <param name="table">The <see cref="Table" /> property of the result.</param>
/// <param name="selectExpression">The <see cref="SelectExpression" /> property of the result.</param>
/// <returns>This expression if no children changed, or an expression with the updated children.</returns>
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);

/// <inheritdoc />
public void Print(ExpressionPrinter expressionPrinter)
Expand Down
12 changes: 8 additions & 4 deletions src/EFCore.Relational/Query/SqlExpressions/ExceptExpression.cs
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ private ExceptExpression(
SelectExpression source1,
SelectExpression source2,
bool distinct,
IEnumerable<IAnnotation>? annotations)
IReadOnlyDictionary<string, IAnnotation>? annotations)
: base(alias, source1, source2, distinct, annotations)
{
}
Expand All @@ -58,12 +58,12 @@ protected override Expression VisitChildren(ExpressionVisitor visitor)
/// <returns>This expression if no children changed, or an expression with the updated children.</returns>
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;

/// <inheritdoc />
protected override TableExpressionBase CreateWithAnnotations(IEnumerable<IAnnotation> annotations)
=> new ExceptExpression(Alias, Source1, Source2, IsDistinct, annotations);
protected override ExceptExpression WithAnnotations(IReadOnlyDictionary<string, IAnnotation> annotations)
=> new(Alias, Source1, Source2, IsDistinct, annotations);

/// <inheritdoc />
public override TableExpressionBase Clone(string? alias, ExpressionVisitor cloningExpressionVisitor)
Expand All @@ -73,6 +73,10 @@ public override TableExpressionBase Clone(string? alias, ExpressionVisitor cloni
(SelectExpression)cloningExpressionVisitor.Visit(Source2),
IsDistinct);

/// <inheritdoc />
public override ExceptExpression WithAlias(string newAlias)
=> new(newAlias, Source1, Source2, IsDistinct);

/// <inheritdoc />
protected override void Print(ExpressionPrinter expressionPrinter)
{
Expand Down
22 changes: 11 additions & 11 deletions src/EFCore.Relational/Query/SqlExpressions/FromSqlExpression.cs
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ private FromSqlExpression(
ITableBase? tableBase,
string sql,
Expression arguments,
IEnumerable<IAnnotation>? annotations)
IReadOnlyDictionary<string, IAnnotation>? annotations)
: base(alias, annotations)
{
Table = tableBase;
Expand All @@ -65,12 +65,8 @@ private FromSqlExpression(
/// <summary>
/// The alias assigned to this table source.
/// </summary>
[NotNull]
public override string? Alias
{
get => base.Alias!;
internal set => base.Alias = value;
}
public override string Alias
=> base.Alias!;

/// <summary>
/// The user-provided custom SQL for the table source.
Expand All @@ -95,20 +91,24 @@ public override string? Alias
/// <returns>This expression if no children changed, or an expression with the updated children.</returns>
public virtual FromSqlExpression Update(Expression arguments)
=> arguments != Arguments
? new FromSqlExpression(Alias, Table, Sql, arguments, GetAnnotations())
? new FromSqlExpression(Alias, Table, Sql, arguments, Annotations)
: this;

/// <inheritdoc />
protected override TableExpressionBase CreateWithAnnotations(IEnumerable<IAnnotation> annotations)
=> new FromSqlExpression(Alias, Table, Sql, Arguments, annotations);
protected override FromSqlExpression WithAnnotations(IReadOnlyDictionary<string, IAnnotation> annotations)
=> new(Alias, Table, Sql, Arguments, annotations);

/// <inheritdoc />
public override FromSqlExpression WithAlias(string newAlias)
=> new(newAlias, Table, Sql, Arguments, Annotations);

/// <inheritdoc />
protected override Expression VisitChildren(ExpressionVisitor visitor)
=> this;

/// <inheritdoc />
public override TableExpressionBase Clone(string? alias, ExpressionVisitor cloningVisitor)
=> new FromSqlExpression(alias!, Table, Sql, Arguments, GetAnnotations());
=> new FromSqlExpression(alias!, Table, Sql, Arguments, Annotations);

/// <inheritdoc />
protected override void Print(ExpressionPrinter expressionPrinter)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ private InnerJoinExpression(
TableExpressionBase table,
SqlExpression joinPredicate,
bool prunable,
IEnumerable<IAnnotation>? annotations)
IReadOnlyDictionary<string, IAnnotation>? annotations)
: base(table, joinPredicate, prunable, annotations)
{
}
Expand All @@ -43,7 +43,7 @@ private InnerJoinExpression(
/// <returns>This expression if no children changed, or an expression with the updated children.</returns>
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;

/// <summary>
Expand All @@ -54,12 +54,12 @@ public override InnerJoinExpression Update(TableExpressionBase table, SqlExpress
/// <returns>This expression if no children changed, or an expression with the updated children.</returns>
public override InnerJoinExpression Update(TableExpressionBase table)
=> table != Table
? new InnerJoinExpression(table, JoinPredicate, IsPrunable, GetAnnotations())
? new InnerJoinExpression(table, JoinPredicate, IsPrunable, Annotations)
: this;

/// <inheritdoc />
protected override TableExpressionBase CreateWithAnnotations(IEnumerable<IAnnotation> annotations)
=> new InnerJoinExpression(Table, JoinPredicate, IsPrunable, annotations);
protected override InnerJoinExpression WithAnnotations(IReadOnlyDictionary<string, IAnnotation> annotations)
=> new(Table, JoinPredicate, IsPrunable, annotations);

/// <inheritdoc />
protected override void Print(ExpressionPrinter expressionPrinter)
Expand Down
Loading