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

Track prunability of inner joins on the expression itself #32674

Merged
merged 1 commit into from
Dec 30, 2023
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
1 change: 1 addition & 0 deletions EFCore.sln.DotSettings
Original file line number Diff line number Diff line change
Expand Up @@ -315,6 +315,7 @@ The .NET Foundation licenses this file to you under the MIT license.
<s:Boolean x:Key="/Default/UserDictionary/Words/=pluralizer/@EntryIndexedValue">True</s:Boolean>
<s:Boolean x:Key="/Default/UserDictionary/Words/=Poolable/@EntryIndexedValue">True</s:Boolean>
<s:Boolean x:Key="/Default/UserDictionary/Words/=Postgre/@EntryIndexedValue">True</s:Boolean>
<s:Boolean x:Key="/Default/UserDictionary/Words/=prunable/@EntryIndexedValue">True</s:Boolean>
<s:Boolean x:Key="/Default/UserDictionary/Words/=pushdown/@EntryIndexedValue">True</s:Boolean>
<s:Boolean x:Key="/Default/UserDictionary/Words/=queryables/@EntryIndexedValue">True</s:Boolean>
<s:Boolean x:Key="/Default/UserDictionary/Words/=remapper/@EntryIndexedValue">True</s:Boolean>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ public CrossApplyExpression(TableExpressionBase table)
}

private CrossApplyExpression(TableExpressionBase table, IEnumerable<IAnnotation>? annotations)
: base(table, annotations)
: base(table, prunable: false, annotations)
{
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ public CrossJoinExpression(TableExpressionBase table)
}

private CrossJoinExpression(TableExpressionBase table, IEnumerable<IAnnotation>? annotations)
: base(table, annotations)
: base(table, prunable: false, annotations)
{
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,16 +19,18 @@ public class InnerJoinExpression : PredicateJoinExpressionBase
/// </summary>
/// <param name="table">A table source to INNER JOIN with.</param>
/// <param name="joinPredicate">A predicate to use for the join.</param>
public InnerJoinExpression(TableExpressionBase table, SqlExpression joinPredicate)
: this(table, joinPredicate, annotations: null)
/// <param name="prunable">Whether this join expression may be pruned if nothing references a column on it.</param>
public InnerJoinExpression(TableExpressionBase table, SqlExpression joinPredicate, bool prunable = false)
: this(table, joinPredicate, prunable, annotations: null)
{
}

private InnerJoinExpression(
TableExpressionBase table,
SqlExpression joinPredicate,
bool prunable,
IEnumerable<IAnnotation>? annotations)
: base(table, joinPredicate, annotations)
: base(table, joinPredicate, prunable, annotations)
{
}

Expand All @@ -41,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, GetAnnotations())
? new InnerJoinExpression(table, joinPredicate, IsPrunable, GetAnnotations())
: this;

/// <summary>
Expand All @@ -52,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, GetAnnotations())
? new InnerJoinExpression(table, JoinPredicate, IsPrunable, GetAnnotations())
: this;

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

/// <inheritdoc />
protected override void Print(ExpressionPrinter expressionPrinter)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,27 +18,26 @@ public abstract class JoinExpressionBase : TableExpressionBase
/// Creates a new instance of the <see cref="JoinExpressionBase" /> class.
/// </summary>
/// <param name="table">A table source to join with.</param>
protected JoinExpressionBase(TableExpressionBase table)
: this(table, annotations: null)
{
}

/// <summary>
/// Creates a new instance of the <see cref="JoinExpressionBase" /> class.
/// </summary>
/// <param name="table">A table source to join with.</param>
/// <param name="prunable">Whether this join expression may be pruned if nothing references a column on it.</param>
/// <param name="annotations">A collection of annotations associated with this expression.</param>
protected JoinExpressionBase(TableExpressionBase table, IEnumerable<IAnnotation>? annotations)
protected JoinExpressionBase(TableExpressionBase table, bool prunable, IEnumerable<IAnnotation>? annotations = null)
: base(alias: null, annotations)
{
Table = table;
IsPrunable = prunable;
}

/// <summary>
/// Gets the underlying table source to join with.
/// </summary>
public virtual TableExpressionBase Table { get; }

/// <summary>
/// Whether this join expression may be pruned if nothing references a column on it. This isn't the case, for example, when an
/// INNER JOIN is used to filter out rows.
/// </summary>
public virtual bool IsPrunable { get; }

/// <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.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,16 +19,18 @@ public class LeftJoinExpression : PredicateJoinExpressionBase
/// </summary>
/// <param name="table">A table source to LEFT JOIN with.</param>
/// <param name="joinPredicate">A predicate to use for the join.</param>
public LeftJoinExpression(TableExpressionBase table, SqlExpression joinPredicate)
: this(table, joinPredicate, annotations: null)
/// <param name="prunable">Whether this join expression may be pruned if nothing references a column on it.</param>
public LeftJoinExpression(TableExpressionBase table, SqlExpression joinPredicate, bool prunable = false)
: this(table, joinPredicate, prunable, annotations: null)
{
}

private LeftJoinExpression(
TableExpressionBase table,
SqlExpression joinPredicate,
IEnumerable<IAnnotation>? annotations)
: base(table, joinPredicate, annotations)
bool prunable,
IEnumerable<IAnnotation>? annotations = null)
: base(table, joinPredicate, prunable, annotations)
{
}

Expand All @@ -41,7 +43,7 @@ private LeftJoinExpression(
/// <returns>This expression if no children changed, or an expression with the updated children.</returns>
public override LeftJoinExpression Update(TableExpressionBase table, SqlExpression joinPredicate)
=> table != Table || joinPredicate != JoinPredicate
? new LeftJoinExpression(table, joinPredicate, GetAnnotations())
? new LeftJoinExpression(table, joinPredicate, IsPrunable, GetAnnotations())
: this;

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

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

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

private OuterApplyExpression(TableExpressionBase table, IEnumerable<IAnnotation>? annotations)
: base(table, annotations)
: base(table, prunable: false, annotations)
{
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,22 +19,14 @@ public abstract class PredicateJoinExpressionBase : JoinExpressionBase
/// </summary>
/// <param name="table">A table source to join with.</param>
/// <param name="joinPredicate">A predicate to use for the join.</param>
protected PredicateJoinExpressionBase(TableExpressionBase table, SqlExpression joinPredicate)
: this(table, joinPredicate, annotations: null)
{
}

/// <summary>
/// Creates a new instance of the <see cref="PredicateJoinExpressionBase" /> class.
/// </summary>
/// <param name="table">A table source to join with.</param>
/// <param name="joinPredicate">A predicate to use for the join.</param>
/// <param name="prunable">Whether this join expression may be pruned if nothing references a column on it.</param>
/// <param name="annotations">A collection of annotations associated with this expression.</param>
protected PredicateJoinExpressionBase(
TableExpressionBase table,
SqlExpression joinPredicate,
IEnumerable<IAnnotation>? annotations)
: base(table, annotations)
bool prunable,
IEnumerable<IAnnotation>? annotations = null)
: base(table, prunable, annotations)
{
JoinPredicate = joinPredicate;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -804,8 +804,6 @@ private sealed class CloningExpressionVisitor : ExpressionVisitor
_mutable = selectExpression._mutable
};

newSelectExpression._removableJoinTables.AddRange(selectExpression._removableJoinTables);

foreach (var kvp in selectExpression._tpcDiscriminatorValues)
{
newSelectExpression._tpcDiscriminatorValues[tpcTablesMap[kvp.Key]] = kvp.Value;
Expand Down
26 changes: 7 additions & 19 deletions src/EFCore.Relational/Query/SqlExpressions/SelectExpression.cs
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,6 @@ public sealed partial class SelectExpression : TableExpressionBase

private readonly List<(ColumnExpression Column, ValueComparer Comparer)> _identifier = new();
private readonly List<(ColumnExpression Column, ValueComparer Comparer)> _childIdentifiers = new();
private readonly List<int> _removableJoinTables = new();

private readonly Dictionary<TpcTablesExpression, (ColumnExpression, List<string>)> _tpcDiscriminatorValues
= new(ReferenceEqualityComparer.Instance);
Expand Down Expand Up @@ -225,8 +224,7 @@ internal SelectExpression(IEntityType entityType, ISqlExpressionFactory sqlExpre
.Zip(keyColumns, sqlExpressionFactory.Equal)
.Aggregate(sqlExpressionFactory.AndAlso);

var joinExpression = new LeftJoinExpression(tableExpression, joinPredicate);
_removableJoinTables.Add(_tables.Count);
var joinExpression = new LeftJoinExpression(tableExpression, joinPredicate, prunable: true);
AddTable(joinExpression, tableReferenceExpression);
}

Expand Down Expand Up @@ -435,8 +433,7 @@ _projectionMapping[new ProjectionMember()] =
.Zip(innerColumns, sqlExpressionFactory.Equal)
.Aggregate(sqlExpressionFactory.AndAlso);

var joinExpression = new InnerJoinExpression(tableExpression, joinPredicate);
_removableJoinTables.Add(_tables.Count);
var joinExpression = new InnerJoinExpression(tableExpression, joinPredicate, prunable: true);
AddTable(joinExpression, tableReferenceExpression);
}
}
Expand Down Expand Up @@ -2496,8 +2493,6 @@ private void ApplySetOperation(SetOperationType setOperationType, SelectExpressi
_projectionMapping.Clear();
select1._identifier.AddRange(_identifier);
_identifier.Clear();
select1._removableJoinTables.AddRange(_removableJoinTables);
_removableJoinTables.Clear();
foreach (var kvp in _tpcDiscriminatorValues)
{
select1._tpcDiscriminatorValues[kvp.Key] = kvp.Value;
Expand Down Expand Up @@ -3045,8 +3040,7 @@ static IReadOnlyDictionary<IProperty, ColumnExpression> GetPropertyExpressions(
.Zip(innerColumns, sqlExpressionFactory.Equal)
.Aggregate(sqlExpressionFactory.AndAlso);

var joinExpression = new LeftJoinExpression(tableExpression, joinPredicate);
selectExpression._removableJoinTables.Add(selectExpression._tables.Count);
var joinExpression = new LeftJoinExpression(tableExpression, joinPredicate, prunable: true);
selectExpression.AddTable(joinExpression, tableReferenceExpression);
}
}
Expand Down Expand Up @@ -3129,8 +3123,7 @@ static IReadOnlyDictionary<IProperty, ColumnExpression> GetPropertyExpressions(
.Zip(innerColumns, sqlExpressionFactory.Equal)
.Aggregate(sqlExpressionFactory.AndAlso);

var joinExpression = new LeftJoinExpression(tableExpression, joinPredicate);
selectExpression._removableJoinTables.Add(selectExpression._tables.Count);
var joinExpression = new LeftJoinExpression(tableExpression, joinPredicate, prunable: true);
selectExpression.AddTable(joinExpression, tableReferenceExpression);
}
}
Expand Down Expand Up @@ -3974,8 +3967,6 @@ private SqlRemappingVisitor PushdownIntoSubqueryInternal(bool liftOrderings = tr
Offset = null;
Limit = null;
_preGroupByIdentifier = null;
subquery._removableJoinTables.AddRange(_removableJoinTables);
_removableJoinTables.Clear();
foreach (var kvp in _tpcDiscriminatorValues)
{
subquery._tpcDiscriminatorValues[kvp.Key] = kvp.Value;
Expand Down Expand Up @@ -4439,11 +4430,9 @@ public void Prune(SqlTreePruner pruningVisitor, bool pruneProjection)

if (!referencedColumnMap.ContainsKey(wrappedTable)
// Note that we only prune joins; pruning the main is more complex because other tables need to unwrap joins to be main.
// TODO: why not CrossApplyExpression/CrossJoin (any join basically)?
&& table is LeftJoinExpression or OuterApplyExpression or InnerJoinExpression
// Only prune InnerJoin if it's in the removable list; since inner joins filter out rows, it may be needed even if it's not
// referenced from anywhere in the query.
&& _removableJoinTables.Contains(i))
// We also only prune joins explicitly marked as prunable; otherwise e.g. an inner join may be needed to filter out rows
// even if no column references it.
&& table is JoinExpressionBase { IsPrunable: true })
{
_tables.RemoveAt(i);
_tableReferences.RemoveAt(i);
Expand Down Expand Up @@ -4821,7 +4810,6 @@ private Expression VisitChildren(ExpressionVisitor visitor, bool updateColumns)
_usedAliases = _usedAliases,
_mutable = false
};
newSelectExpression._removableJoinTables.AddRange(_removableJoinTables);
foreach (var kvp in newTpcDiscriminatorValues)
{
newSelectExpression._tpcDiscriminatorValues[kvp.Key] = kvp.Value;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,22 +17,12 @@ public abstract class TableExpressionBase : Expression, IPrintableExpression
{
private readonly IReadOnlyDictionary<string, IAnnotation>? _annotations;

/// <summary>
/// Creates a new instance of the <see cref="TableExpressionBase" /> class.
/// </summary>
/// <param name="alias">A string alias for the table source.</param>
protected TableExpressionBase(string? alias)
: this(alias, annotations: null)
{
Alias = alias;
}

/// <summary>
/// Creates a new instance of the <see cref="TableExpressionBase" /> class.
/// </summary>
/// <param name="alias">A string alias for the table source.</param>
/// <param name="annotations">A collection of annotations associated with this expression.</param>
protected TableExpressionBase(string? alias, IEnumerable<IAnnotation>? annotations)
protected TableExpressionBase(string? alias, IEnumerable<IAnnotation>? annotations = null)
{
Alias = alias;

Expand Down
Loading