Skip to content

Commit 6548216

Browse files
author
James Cor
committed
incorporate swaps
1 parent c913ae2 commit 6548216

File tree

1 file changed

+31
-20
lines changed

1 file changed

+31
-20
lines changed

sql/analyzer/replace_sort.go

Lines changed: 31 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -154,16 +154,16 @@ func replaceIdxSortHelper(ctx *sql.Context, scope *plan.Scope, node sql.Node, so
154154
*plan.Project, *plan.Filter, *plan.Limit, *plan.Offset, *plan.Distinct, *plan.TableAlias:
155155
newChildren[i], same, err = replaceIdxSortHelper(ctx, scope, child, sortNode)
156156
case *plan.JoinNode:
157-
// Merge Joins assume that left and right are sorted
158-
// Cross Joins and Inner Joins don't care if
159-
if !c.JoinType().IsMerge() && !c.JoinType().IsCross() && !c.JoinType().IsInner() {
160-
continue
161-
}
162157
// It's (probably) not possible to have Sort as child of Join without Subquery/SubqueryAlias,
163158
// and in the case where there is a Subq/SQA it's taken care of through finalizeSubqueries
164159
if sortNode == nil {
165160
continue
166161
}
162+
// Merge Joins assume that left and right are sorted
163+
// Cross Joins and Inner Joins are valid for sort removal if left child is sorted
164+
if !c.JoinType().IsMerge() && !c.JoinType().IsCross() && !c.JoinType().IsInner() {
165+
continue
166+
}
167167
newLeft, sameLeft, errLeft := replaceIdxSortHelper(ctx, scope, c.Left(), sortNode)
168168
if errLeft != nil {
169169
return nil, transform.SameTree, errLeft
@@ -172,6 +172,7 @@ func replaceIdxSortHelper(ctx *sql.Context, scope *plan.Scope, node sql.Node, so
172172
if errRight != nil {
173173
return nil, transform.SameTree, errRight
174174
}
175+
// Nothing replaced, so nothing to do
175176
if sameLeft && sameRight {
176177
continue
177178
}
@@ -183,24 +184,34 @@ func replaceIdxSortHelper(ctx *sql.Context, scope *plan.Scope, node sql.Node, so
183184
c.IsReversed = isReversed
184185
continue
185186
}
186-
// If only one side has been replaced, we need to check if the other side can be reversed
187-
if (sameLeft != sameRight) && isReversed {
188-
// If descending, then both Indexes must be reversed
189-
if sameLeft {
190-
newLeft, same, err = buildReverseIndexedTable(ctx, newLeft)
191-
} else if sameRight {
192-
newRight, same, err = buildReverseIndexedTable(ctx, newRight)
193-
}
194-
if err != nil {
195-
return nil, transform.SameTree, err
187+
if c.JoinType().IsCross() || c.JoinType().IsInner() {
188+
// For cross joins and inner joins, if the right child is sorted, we need to swap
189+
if !sameRight {
190+
// Rule eraseProjection will drop any Projections that are now unnecessary.
191+
// Rule fixExecIndexes will fix any existing Projection GetField indexes.
192+
newLeft, newRight = newRight, newLeft
196193
}
197-
// If we could not replace the IndexedTableAccess with a reversed one (due to lack of reversible index)
198-
// same = true, so just continue
199-
if same {
200-
continue
194+
} else {
195+
// If only one side has been replaced, we need to check if the other side can be reversed
196+
if (sameLeft != sameRight) && isReversed {
197+
// If descending, then both Indexes must be reversed
198+
if sameLeft {
199+
newLeft, same, err = buildReverseIndexedTable(ctx, newLeft)
200+
} else if sameRight {
201+
newRight, same, err = buildReverseIndexedTable(ctx, newRight)
202+
}
203+
if err != nil {
204+
return nil, transform.SameTree, err
205+
}
206+
// If we could not replace the IndexedTableAccess with a reversed one (due to lack of reversible index)
207+
// same = true, so just continue
208+
if same {
209+
continue
210+
}
211+
c.IsReversed = true
201212
}
202-
c.IsReversed = true
203213
}
214+
204215
newChildren[i], err = c.WithChildren(newLeft, newRight)
205216
if err != nil {
206217
return nil, transform.SameTree, err

0 commit comments

Comments
 (0)