Skip to content

Commit aa91f3b

Browse files
authored
Merge pull request #3284 from dolthub/angela/checksum_outermost
Use IndexedTableAccess for Sorts over a SubqueryAlias
2 parents c8d8087 + 6f580bf commit aa91f3b

File tree

4 files changed

+109
-33
lines changed

4 files changed

+109
-33
lines changed

enginetest/enginetests.go

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -4901,10 +4901,6 @@ func TestSessionSelectLimit(t *testing.T, harness Harness) {
49014901
Query: "SELECT i FROM (SELECT i FROM mytable ORDER BY i DESC) t ORDER BY i LIMIT 3",
49024902
Expected: []sql.Row{{1}, {2}, {3}},
49034903
},
4904-
{
4905-
Query: "SELECT i FROM (SELECT i FROM mytable ORDER BY i DESC) t ORDER BY i LIMIT 3",
4906-
Expected: []sql.Row{{1}, {2}, {3}},
4907-
},
49084904
{
49094905
Query: "select count(*), y from a group by y;",
49104906
Expected: []sql.Row{{2, 1}, {3, 2}},

enginetest/queries/query_plans.go

Lines changed: 70 additions & 26 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

enginetest/queries/tpch_plans.go

Lines changed: 1 addition & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

sql/analyzer/replace_sort.go

Lines changed: 38 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -52,8 +52,10 @@ func replaceIdxSortHelper(ctx *sql.Context, scope *plan.Scope, node sql.Node, so
5252
if hasOverlapping(sfExprs, mysqlRanges) {
5353
return n, transform.SameTree, nil
5454
}
55+
56+
isReverse := sortNode.SortFields[0].Order == sql.Descending
5557
// if the lookup does not need any reversing, do nothing
56-
if sortNode.SortFields[0].Order != sql.Descending {
58+
if (isReverse && lookup.IsReverse) || (!isReverse && !lookup.IsReverse) {
5759
return n, transform.NewTree, nil
5860
}
5961

@@ -67,7 +69,7 @@ func replaceIdxSortHelper(ctx *sql.Context, scope *plan.Scope, node sql.Node, so
6769
lookup.IsPointLookup,
6870
lookup.IsEmptyRange,
6971
lookup.IsSpatialLookup,
70-
true,
72+
isReverse,
7173
)
7274
newIdxTbl, err := plan.NewStaticIndexedAccessForTableNode(ctx, n.TableNode, lookup)
7375
if err != nil {
@@ -154,6 +156,40 @@ func replaceIdxSortHelper(ctx *sql.Context, scope *plan.Scope, node sql.Node, so
154156
case *plan.Sort, *plan.IndexedTableAccess, *plan.ResolvedTable,
155157
*plan.Project, *plan.Filter, *plan.Limit, *plan.Offset, *plan.Distinct, *plan.TableAlias:
156158
newChildren[i], same, err = replaceIdxSortHelper(ctx, scope, child, sortNode)
159+
case *plan.SubqueryAlias:
160+
if sortNode == nil {
161+
continue
162+
}
163+
sortFields := make([]sql.SortField, len(sortNode.SortFields))
164+
sameSortFields := true
165+
for i, sortField := range sortNode.SortFields {
166+
col, sameExpr, _ := transform.Expr(sortField.Column, func(e sql.Expression) (sql.Expression, transform.TreeIdentity, error) {
167+
if gt, ok := e.(*expression.GetField); ok {
168+
if gf, ok := c.ScopeMapping[gt.Id()]; ok {
169+
return gf, transform.NewTree, nil
170+
}
171+
}
172+
return e, transform.SameTree, nil
173+
})
174+
if sameExpr {
175+
sortFields[i] = sortField
176+
} else {
177+
sameSortFields = false
178+
col2, _ := col.(sql.Expression2)
179+
sortFields[i] = sql.SortField{
180+
Column: col,
181+
Column2: col2,
182+
NullOrdering: sortField.NullOrdering,
183+
Order: sortField.Order,
184+
}
185+
}
186+
}
187+
if !sameSortFields {
188+
// The Sort node is used to find table aliases, but table aliases can't be found inside SubqueryAlias
189+
// nodes, so we need to construct a new Sort node with the SubqueryAlias's child
190+
newSort := plan.NewSort(sortFields, c.Child)
191+
newChildren[i], same, err = replaceIdxSortHelper(ctx, scope, child, newSort)
192+
}
157193
case *plan.JoinNode:
158194
// It's (probably) not possible to have Sort as child of Join without Subquery/SubqueryAlias,
159195
// and in the case where there is a Subq/SQA it's taken care of through finalizeSubqueries

0 commit comments

Comments
 (0)