diff --git a/EFCore.sln.DotSettings b/EFCore.sln.DotSettings index 85601ea8df9..6e44bcd4b12 100644 --- a/EFCore.sln.DotSettings +++ b/EFCore.sln.DotSettings @@ -315,6 +315,7 @@ The .NET Foundation licenses this file to you under the MIT license. True True True + True True True True diff --git a/src/EFCore.Relational/Query/SqlExpressions/CrossApplyExpression.cs b/src/EFCore.Relational/Query/SqlExpressions/CrossApplyExpression.cs index c1ba6d964eb..3263c42e759 100644 --- a/src/EFCore.Relational/Query/SqlExpressions/CrossApplyExpression.cs +++ b/src/EFCore.Relational/Query/SqlExpressions/CrossApplyExpression.cs @@ -24,7 +24,7 @@ public CrossApplyExpression(TableExpressionBase table) } private CrossApplyExpression(TableExpressionBase table, IEnumerable? annotations) - : base(table, annotations) + : base(table, prunable: false, annotations) { } diff --git a/src/EFCore.Relational/Query/SqlExpressions/CrossJoinExpression.cs b/src/EFCore.Relational/Query/SqlExpressions/CrossJoinExpression.cs index 89f87da89dc..80b6de42e38 100644 --- a/src/EFCore.Relational/Query/SqlExpressions/CrossJoinExpression.cs +++ b/src/EFCore.Relational/Query/SqlExpressions/CrossJoinExpression.cs @@ -24,7 +24,7 @@ public CrossJoinExpression(TableExpressionBase table) } private CrossJoinExpression(TableExpressionBase table, IEnumerable? annotations) - : base(table, annotations) + : base(table, prunable: false, annotations) { } diff --git a/src/EFCore.Relational/Query/SqlExpressions/InnerJoinExpression.cs b/src/EFCore.Relational/Query/SqlExpressions/InnerJoinExpression.cs index e43e83fc52e..54ba76bf605 100644 --- a/src/EFCore.Relational/Query/SqlExpressions/InnerJoinExpression.cs +++ b/src/EFCore.Relational/Query/SqlExpressions/InnerJoinExpression.cs @@ -19,16 +19,18 @@ public class InnerJoinExpression : PredicateJoinExpressionBase /// /// A table source to INNER JOIN with. /// A predicate to use for the join. - public InnerJoinExpression(TableExpressionBase table, SqlExpression joinPredicate) - : this(table, joinPredicate, annotations: null) + /// Whether this join expression may be pruned if nothing references a column on it. + public InnerJoinExpression(TableExpressionBase table, SqlExpression joinPredicate, bool prunable = false) + : this(table, joinPredicate, prunable, annotations: null) { } private InnerJoinExpression( TableExpressionBase table, SqlExpression joinPredicate, + bool prunable, IEnumerable? annotations) - : base(table, joinPredicate, annotations) + : base(table, joinPredicate, prunable, annotations) { } @@ -41,7 +43,7 @@ private InnerJoinExpression( /// This expression if no children changed, or an expression with the updated children. public override InnerJoinExpression Update(TableExpressionBase table, SqlExpression joinPredicate) => table != Table || joinPredicate != JoinPredicate - ? new InnerJoinExpression(table, joinPredicate, GetAnnotations()) + ? new InnerJoinExpression(table, joinPredicate, IsPrunable, GetAnnotations()) : this; /// @@ -52,12 +54,12 @@ public override InnerJoinExpression Update(TableExpressionBase table, SqlExpress /// This expression if no children changed, or an expression with the updated children. public override InnerJoinExpression Update(TableExpressionBase table) => table != Table - ? new InnerJoinExpression(table, JoinPredicate, GetAnnotations()) + ? new InnerJoinExpression(table, JoinPredicate, IsPrunable, GetAnnotations()) : this; /// protected override TableExpressionBase CreateWithAnnotations(IEnumerable annotations) - => new InnerJoinExpression(Table, JoinPredicate, annotations); + => new InnerJoinExpression(Table, JoinPredicate, IsPrunable, annotations); /// protected override void Print(ExpressionPrinter expressionPrinter) diff --git a/src/EFCore.Relational/Query/SqlExpressions/JoinExpressionBase.cs b/src/EFCore.Relational/Query/SqlExpressions/JoinExpressionBase.cs index 15fae8a45db..ba221e7ed88 100644 --- a/src/EFCore.Relational/Query/SqlExpressions/JoinExpressionBase.cs +++ b/src/EFCore.Relational/Query/SqlExpressions/JoinExpressionBase.cs @@ -18,20 +18,13 @@ public abstract class JoinExpressionBase : TableExpressionBase /// Creates a new instance of the class. /// /// A table source to join with. - protected JoinExpressionBase(TableExpressionBase table) - : this(table, annotations: null) - { - } - - /// - /// Creates a new instance of the class. - /// - /// A table source to join with. + /// Whether this join expression may be pruned if nothing references a column on it. /// A collection of annotations associated with this expression. - protected JoinExpressionBase(TableExpressionBase table, IEnumerable? annotations) + protected JoinExpressionBase(TableExpressionBase table, bool prunable, IEnumerable? annotations = null) : base(alias: null, annotations) { Table = table; + IsPrunable = prunable; } /// @@ -39,6 +32,12 @@ protected JoinExpressionBase(TableExpressionBase table, IEnumerable /// public virtual TableExpressionBase Table { get; } + /// + /// 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. + /// + public virtual bool IsPrunable { get; } + /// /// 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. diff --git a/src/EFCore.Relational/Query/SqlExpressions/LeftJoinExpression.cs b/src/EFCore.Relational/Query/SqlExpressions/LeftJoinExpression.cs index f35ae5b2eb7..24c1725f9d0 100644 --- a/src/EFCore.Relational/Query/SqlExpressions/LeftJoinExpression.cs +++ b/src/EFCore.Relational/Query/SqlExpressions/LeftJoinExpression.cs @@ -19,16 +19,18 @@ public class LeftJoinExpression : PredicateJoinExpressionBase /// /// A table source to LEFT JOIN with. /// A predicate to use for the join. - public LeftJoinExpression(TableExpressionBase table, SqlExpression joinPredicate) - : this(table, joinPredicate, annotations: null) + /// Whether this join expression may be pruned if nothing references a column on it. + public LeftJoinExpression(TableExpressionBase table, SqlExpression joinPredicate, bool prunable = false) + : this(table, joinPredicate, prunable, annotations: null) { } private LeftJoinExpression( TableExpressionBase table, SqlExpression joinPredicate, - IEnumerable? annotations) - : base(table, joinPredicate, annotations) + bool prunable, + IEnumerable? annotations = null) + : base(table, joinPredicate, prunable, annotations) { } @@ -41,7 +43,7 @@ private LeftJoinExpression( /// This expression if no children changed, or an expression with the updated children. public override LeftJoinExpression Update(TableExpressionBase table, SqlExpression joinPredicate) => table != Table || joinPredicate != JoinPredicate - ? new LeftJoinExpression(table, joinPredicate, GetAnnotations()) + ? new LeftJoinExpression(table, joinPredicate, IsPrunable, GetAnnotations()) : this; /// @@ -52,12 +54,12 @@ public override LeftJoinExpression Update(TableExpressionBase table, SqlExpressi /// This expression if no children changed, or an expression with the updated children. public override LeftJoinExpression Update(TableExpressionBase table) => table != Table - ? new LeftJoinExpression(table, JoinPredicate, GetAnnotations()) + ? new LeftJoinExpression(table, JoinPredicate, IsPrunable, GetAnnotations()) : this; /// protected override TableExpressionBase CreateWithAnnotations(IEnumerable annotations) - => new LeftJoinExpression(Table, JoinPredicate, annotations); + => new LeftJoinExpression(Table, JoinPredicate, IsPrunable, annotations); /// protected override void Print(ExpressionPrinter expressionPrinter) diff --git a/src/EFCore.Relational/Query/SqlExpressions/OuterApplyExpression.cs b/src/EFCore.Relational/Query/SqlExpressions/OuterApplyExpression.cs index 7f20fd74494..74940bebbfe 100644 --- a/src/EFCore.Relational/Query/SqlExpressions/OuterApplyExpression.cs +++ b/src/EFCore.Relational/Query/SqlExpressions/OuterApplyExpression.cs @@ -24,7 +24,7 @@ public OuterApplyExpression(TableExpressionBase table) } private OuterApplyExpression(TableExpressionBase table, IEnumerable? annotations) - : base(table, annotations) + : base(table, prunable: false, annotations) { } diff --git a/src/EFCore.Relational/Query/SqlExpressions/PredicateJoinExpressionBase.cs b/src/EFCore.Relational/Query/SqlExpressions/PredicateJoinExpressionBase.cs index 1c8b1283bcc..815e596fac2 100644 --- a/src/EFCore.Relational/Query/SqlExpressions/PredicateJoinExpressionBase.cs +++ b/src/EFCore.Relational/Query/SqlExpressions/PredicateJoinExpressionBase.cs @@ -19,22 +19,14 @@ public abstract class PredicateJoinExpressionBase : JoinExpressionBase /// /// A table source to join with. /// A predicate to use for the join. - protected PredicateJoinExpressionBase(TableExpressionBase table, SqlExpression joinPredicate) - : this(table, joinPredicate, annotations: null) - { - } - - /// - /// Creates a new instance of the class. - /// - /// A table source to join with. - /// A predicate to use for the join. + /// Whether this join expression may be pruned if nothing references a column on it. /// A collection of annotations associated with this expression. protected PredicateJoinExpressionBase( TableExpressionBase table, SqlExpression joinPredicate, - IEnumerable? annotations) - : base(table, annotations) + bool prunable, + IEnumerable? annotations = null) + : base(table, prunable, annotations) { JoinPredicate = joinPredicate; } diff --git a/src/EFCore.Relational/Query/SqlExpressions/SelectExpression.Helper.cs b/src/EFCore.Relational/Query/SqlExpressions/SelectExpression.Helper.cs index 5a0c5a25474..bfbcae94c6d 100644 --- a/src/EFCore.Relational/Query/SqlExpressions/SelectExpression.Helper.cs +++ b/src/EFCore.Relational/Query/SqlExpressions/SelectExpression.Helper.cs @@ -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; diff --git a/src/EFCore.Relational/Query/SqlExpressions/SelectExpression.cs b/src/EFCore.Relational/Query/SqlExpressions/SelectExpression.cs index 925ff9ce612..546e0637f07 100644 --- a/src/EFCore.Relational/Query/SqlExpressions/SelectExpression.cs +++ b/src/EFCore.Relational/Query/SqlExpressions/SelectExpression.cs @@ -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 _removableJoinTables = new(); private readonly Dictionary)> _tpcDiscriminatorValues = new(ReferenceEqualityComparer.Instance); @@ -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); } @@ -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); } } @@ -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; @@ -3045,8 +3040,7 @@ static IReadOnlyDictionary 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); } } @@ -3129,8 +3123,7 @@ static IReadOnlyDictionary 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); } } @@ -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; @@ -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); @@ -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; diff --git a/src/EFCore.Relational/Query/SqlExpressions/TableExpressionBase.cs b/src/EFCore.Relational/Query/SqlExpressions/TableExpressionBase.cs index fa1c22b28e3..63a0ca7f5a6 100644 --- a/src/EFCore.Relational/Query/SqlExpressions/TableExpressionBase.cs +++ b/src/EFCore.Relational/Query/SqlExpressions/TableExpressionBase.cs @@ -17,22 +17,12 @@ public abstract class TableExpressionBase : Expression, IPrintableExpression { private readonly IReadOnlyDictionary? _annotations; - /// - /// Creates a new instance of the class. - /// - /// A string alias for the table source. - protected TableExpressionBase(string? alias) - : this(alias, annotations: null) - { - Alias = alias; - } - /// /// Creates a new instance of the class. /// /// A string alias for the table source. /// A collection of annotations associated with this expression. - protected TableExpressionBase(string? alias, IEnumerable? annotations) + protected TableExpressionBase(string? alias, IEnumerable? annotations = null) { Alias = alias;