Skip to content

Commit

Permalink
Track prunability of inner joins on the expression itself
Browse files Browse the repository at this point in the history
Instead of on a private list on SelectExpression.
Part of dotnet#31049
  • Loading branch information
roji committed Dec 29, 2023
1 parent 651d5b9 commit ff71406
Show file tree
Hide file tree
Showing 11 changed files with 42 additions and 70 deletions.
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
19 changes: 9 additions & 10 deletions src/EFCore.Relational/Query/SqlExpressions/JoinExpressionBase.cs
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
16 changes: 9 additions & 7 deletions src/EFCore.Relational/Query/SqlExpressions/LeftJoinExpression.cs
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

0 comments on commit ff71406

Please sign in to comment.