From f5153236c2eceff7acdefc069b3f93db38a0eb45 Mon Sep 17 00:00:00 2001 From: Angela Xie Date: Tue, 28 Oct 2025 16:27:27 -0700 Subject: [PATCH 1/5] replace sort nodes through subquery aliases --- sql/analyzer/replace_sort.go | 30 ++++++++++++++++++++++++++++++ sql/plan/sort.go | 6 ++++++ 2 files changed, 36 insertions(+) diff --git a/sql/analyzer/replace_sort.go b/sql/analyzer/replace_sort.go index 8aad71c692..2734a6d932 100644 --- a/sql/analyzer/replace_sort.go +++ b/sql/analyzer/replace_sort.go @@ -154,6 +154,36 @@ func replaceIdxSortHelper(ctx *sql.Context, scope *plan.Scope, node sql.Node, so case *plan.Sort, *plan.IndexedTableAccess, *plan.ResolvedTable, *plan.Project, *plan.Filter, *plan.Limit, *plan.Offset, *plan.Distinct, *plan.TableAlias: newChildren[i], same, err = replaceIdxSortHelper(ctx, scope, child, sortNode) + case *plan.SubqueryAlias: + if sortNode == nil { + continue + } + sortFields := sortNode.SortFields + for i, sortField := range sortNode.SortFields { + col, sameExpr, _ := transform.Expr(sortField.Column, func(e sql.Expression) (sql.Expression, transform.TreeIdentity, error) { + if gt, ok := e.(*expression.GetField); ok { + if gf, ok := c.ScopeMapping[gt.Id()]; ok { + return gf, transform.NewTree, nil + } + } + return e, transform.SameTree, nil + }) + if !sameExpr { + col2, _ := col.(sql.Expression2) + sortFields[i] = sql.SortField{ + Column: col, + Column2: col2, + NullOrdering: sortField.NullOrdering, + Order: sortField.Order, + } + } + } + newSort := sortNode.WithSortFields(sortFields) + newSort.Child = c.Child + if err != nil { + return nil, transform.SameTree, err + } + newChildren[i], same, err = replaceIdxSortHelper(ctx, scope, child, newSort) case *plan.JoinNode: // It's (probably) not possible to have Sort as child of Join without Subquery/SubqueryAlias, // and in the case where there is a Subq/SQA it's taken care of through finalizeSubqueries diff --git a/sql/plan/sort.go b/sql/plan/sort.go index c780e602c1..435f3c9d9a 100644 --- a/sql/plan/sort.go +++ b/sql/plan/sort.go @@ -115,6 +115,12 @@ func (s *Sort) WithExpressions(exprs ...sql.Expression) (sql.Node, error) { return NewSort(fields, s.Child), nil } +func (s *Sort) WithSortFields(sf sql.SortFields) *Sort { + ret := *s + ret.SortFields = sf + return &ret +} + func (s *Sort) GetSortFields() sql.SortFields { return s.SortFields } From d628a3143087cdaf18c017af7046f21edc3431a3 Mon Sep 17 00:00:00 2001 From: Angela Xie Date: Tue, 28 Oct 2025 16:30:14 -0700 Subject: [PATCH 2/5] clean up --- sql/analyzer/replace_sort.go | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/sql/analyzer/replace_sort.go b/sql/analyzer/replace_sort.go index 2734a6d932..7fcc1b83d9 100644 --- a/sql/analyzer/replace_sort.go +++ b/sql/analyzer/replace_sort.go @@ -179,10 +179,8 @@ func replaceIdxSortHelper(ctx *sql.Context, scope *plan.Scope, node sql.Node, so } } newSort := sortNode.WithSortFields(sortFields) + // The sort node is used to find table aliases and we need to get the table aliases inside the SubqueryAlias newSort.Child = c.Child - if err != nil { - return nil, transform.SameTree, err - } newChildren[i], same, err = replaceIdxSortHelper(ctx, scope, child, newSort) case *plan.JoinNode: // It's (probably) not possible to have Sort as child of Join without Subquery/SubqueryAlias, From 0de81e43201206f9234ef9d41d067da7cd910a9f Mon Sep 17 00:00:00 2001 From: Angela Xie Date: Wed, 29 Oct 2025 16:21:50 -0700 Subject: [PATCH 3/5] fix broken tests --- enginetest/enginetests.go | 4 ---- sql/analyzer/replace_sort.go | 24 ++++++++++++++++-------- sql/plan/sort.go | 6 ------ 3 files changed, 16 insertions(+), 18 deletions(-) diff --git a/enginetest/enginetests.go b/enginetest/enginetests.go index 04f8994c78..7997aff395 100644 --- a/enginetest/enginetests.go +++ b/enginetest/enginetests.go @@ -4901,10 +4901,6 @@ func TestSessionSelectLimit(t *testing.T, harness Harness) { Query: "SELECT i FROM (SELECT i FROM mytable ORDER BY i DESC) t ORDER BY i LIMIT 3", Expected: []sql.Row{{1}, {2}, {3}}, }, - { - Query: "SELECT i FROM (SELECT i FROM mytable ORDER BY i DESC) t ORDER BY i LIMIT 3", - Expected: []sql.Row{{1}, {2}, {3}}, - }, { Query: "select count(*), y from a group by y;", Expected: []sql.Row{{2, 1}, {3, 2}}, diff --git a/sql/analyzer/replace_sort.go b/sql/analyzer/replace_sort.go index 7fcc1b83d9..7a2e4b3ff7 100644 --- a/sql/analyzer/replace_sort.go +++ b/sql/analyzer/replace_sort.go @@ -52,8 +52,10 @@ func replaceIdxSortHelper(ctx *sql.Context, scope *plan.Scope, node sql.Node, so if hasOverlapping(sfExprs, mysqlRanges) { return n, transform.SameTree, nil } + + isReverse := sortNode.SortFields[0].Order == sql.Descending // if the lookup does not need any reversing, do nothing - if sortNode.SortFields[0].Order != sql.Descending { + if (isReverse && lookup.IsReverse) || (!isReverse && !lookup.IsReverse) { return n, transform.NewTree, nil } @@ -67,7 +69,7 @@ func replaceIdxSortHelper(ctx *sql.Context, scope *plan.Scope, node sql.Node, so lookup.IsPointLookup, lookup.IsEmptyRange, lookup.IsSpatialLookup, - true, + isReverse, ) newIdxTbl, err := plan.NewStaticIndexedAccessForTableNode(ctx, n.TableNode, lookup) if err != nil { @@ -158,7 +160,8 @@ func replaceIdxSortHelper(ctx *sql.Context, scope *plan.Scope, node sql.Node, so if sortNode == nil { continue } - sortFields := sortNode.SortFields + sortFields := make([]sql.SortField, len(sortNode.SortFields)) + sameSortFields := true for i, sortField := range sortNode.SortFields { col, sameExpr, _ := transform.Expr(sortField.Column, func(e sql.Expression) (sql.Expression, transform.TreeIdentity, error) { if gt, ok := e.(*expression.GetField); ok { @@ -168,7 +171,10 @@ func replaceIdxSortHelper(ctx *sql.Context, scope *plan.Scope, node sql.Node, so } return e, transform.SameTree, nil }) - if !sameExpr { + if sameExpr { + sortFields[i] = sortField + } else { + sameSortFields = false col2, _ := col.(sql.Expression2) sortFields[i] = sql.SortField{ Column: col, @@ -178,10 +184,12 @@ func replaceIdxSortHelper(ctx *sql.Context, scope *plan.Scope, node sql.Node, so } } } - newSort := sortNode.WithSortFields(sortFields) - // The sort node is used to find table aliases and we need to get the table aliases inside the SubqueryAlias - newSort.Child = c.Child - newChildren[i], same, err = replaceIdxSortHelper(ctx, scope, child, newSort) + if !sameSortFields { + // The Sort node is used to find table aliases, but table aliases can't be found inside SubqueryAlias + // nodes, so we need to construct a new Sort node with the SubqueryAlias's child + newSort := plan.NewSort(sortFields, c.Child) + newChildren[i], same, err = replaceIdxSortHelper(ctx, scope, child, newSort) + } case *plan.JoinNode: // It's (probably) not possible to have Sort as child of Join without Subquery/SubqueryAlias, // and in the case where there is a Subq/SQA it's taken care of through finalizeSubqueries diff --git a/sql/plan/sort.go b/sql/plan/sort.go index 435f3c9d9a..c780e602c1 100644 --- a/sql/plan/sort.go +++ b/sql/plan/sort.go @@ -115,12 +115,6 @@ func (s *Sort) WithExpressions(exprs ...sql.Expression) (sql.Node, error) { return NewSort(fields, s.Child), nil } -func (s *Sort) WithSortFields(sf sql.SortFields) *Sort { - ret := *s - ret.SortFields = sf - return &ret -} - func (s *Sort) GetSortFields() sql.SortFields { return s.SortFields } From 3caef1677a013388f18ccba4644009e33dcb451a Mon Sep 17 00:00:00 2001 From: Angela Xie Date: Wed, 29 Oct 2025 16:40:43 -0700 Subject: [PATCH 4/5] update query plans --- enginetest/queries/query_plans.go | 85 ++++++++++++++++++++----------- enginetest/queries/tpch_plans.go | 2 +- 2 files changed, 57 insertions(+), 30 deletions(-) diff --git a/enginetest/queries/query_plans.go b/enginetest/queries/query_plans.go index 72da79345d..4e6dc2b356 100644 --- a/enginetest/queries/query_plans.go +++ b/enginetest/queries/query_plans.go @@ -3568,43 +3568,70 @@ Select * from ( Query: `SELECT a FROM (select i,s FROM mytable) mt (a,b) order by 1;`, ExpectedPlan: "Project\n" + " ├─ columns: [mt.a:0!null]\n" + - " └─ Sort(mt.a:0!null ASC nullsFirst)\n" + - " └─ SubqueryAlias\n" + - " ├─ name: mt\n" + - " ├─ outerVisibility: false\n" + - " ├─ isLateral: false\n" + - " ├─ cacheable: true\n" + - " ├─ colSet: (3,4)\n" + - " ├─ tableId: 2\n" + + " └─ SubqueryAlias\n" + + " ├─ name: mt\n" + + " ├─ outerVisibility: false\n" + + " ├─ isLateral: false\n" + + " ├─ cacheable: true\n" + + " ├─ colSet: (3,4)\n" + + " ├─ tableId: 2\n" + + " └─ IndexedTableAccess(mytable)\n" + + " ├─ index: [mytable.i]\n" + + " ├─ static: [{[NULL, ∞)}]\n" + + " ├─ colSet: (1,2)\n" + + " ├─ tableId: 1\n" + " └─ Table\n" + " ├─ name: mytable\n" + - " ├─ columns: [i s]\n" + - " ├─ colSet: (1,2)\n" + - " └─ tableId: 1\n" + + " └─ columns: [i s]\n" + "", ExpectedEstimates: "Project\n" + " ├─ columns: [mt.a]\n" + - " └─ Sort(mt.a ASC)\n" + - " └─ SubqueryAlias\n" + - " ├─ name: mt\n" + - " ├─ outerVisibility: false\n" + - " ├─ isLateral: false\n" + - " ├─ cacheable: true\n" + - " └─ Table\n" + - " ├─ name: mytable\n" + - " └─ columns: [i s]\n" + + " └─ SubqueryAlias\n" + + " ├─ name: mt\n" + + " ├─ outerVisibility: false\n" + + " ├─ isLateral: false\n" + + " ├─ cacheable: true\n" + + " └─ IndexedTableAccess(mytable)\n" + + " ├─ index: [mytable.i]\n" + + " ├─ filters: [{[NULL, ∞)}]\n" + + " └─ columns: [i s]\n" + "", ExpectedAnalysis: "Project\n" + " ├─ columns: [mt.a]\n" + - " └─ Sort(mt.a ASC)\n" + - " └─ SubqueryAlias\n" + - " ├─ name: mt\n" + - " ├─ outerVisibility: false\n" + - " ├─ isLateral: false\n" + - " ├─ cacheable: true\n" + - " └─ Table\n" + - " ├─ name: mytable\n" + - " └─ columns: [i s]\n" + + " └─ SubqueryAlias\n" + + " ├─ name: mt\n" + + " ├─ outerVisibility: false\n" + + " ├─ isLateral: false\n" + + " ├─ cacheable: true\n" + + " └─ IndexedTableAccess(mytable)\n" + + " ├─ index: [mytable.i]\n" + + " ├─ filters: [{[NULL, ∞)}]\n" + + " └─ columns: [i s]\n" + + "", + }, + { + Query: `SELECT * FROM (select i,s FROM mytable) mt order by i;`, + ExpectedPlan: "TableAlias(mt)\n" + + " └─ IndexedTableAccess(mytable)\n" + + " ├─ index: [mytable.i]\n" + + " ├─ static: [{[NULL, ∞)}]\n" + + " ├─ colSet: (1,2)\n" + + " ├─ tableId: 1\n" + + " └─ Table\n" + + " ├─ name: mytable\n" + + " └─ columns: [i s]\n" + + "", + ExpectedEstimates: "TableAlias(mt)\n" + + " └─ IndexedTableAccess(mytable)\n" + + " ├─ index: [mytable.i]\n" + + " ├─ filters: [{[NULL, ∞)}]\n" + + " └─ columns: [i s]\n" + + "", + ExpectedAnalysis: "TableAlias(mt)\n" + + " └─ IndexedTableAccess(mytable)\n" + + " ├─ index: [mytable.i]\n" + + " ├─ filters: [{[NULL, ∞)}]\n" + + " └─ columns: [i s]\n" + "", }, { diff --git a/enginetest/queries/tpch_plans.go b/enginetest/queries/tpch_plans.go index 29e7aa5ea4..e4c64805dd 100644 --- a/enginetest/queries/tpch_plans.go +++ b/enginetest/queries/tpch_plans.go @@ -3457,7 +3457,7 @@ order by " │ ├─ columns: [avg(customer.c_acctbal):8->avg(c_acctbal):0]\n" + " │ └─ GroupBy\n" + " │ ├─ select: AVG(customer.c_acctbal:9!null)\n" + - " │ ├─ group: \n" + // printer adds space sep always + " │ ├─ group: \n" + " │ └─ Filter\n" + " │ ├─ AND\n" + " │ │ ├─ GreaterThan\n" + From 6f580bf58af93d9d37796fee616afc4d71351987 Mon Sep 17 00:00:00 2001 From: Angela Xie Date: Wed, 29 Oct 2025 16:52:02 -0700 Subject: [PATCH 5/5] better query plan test --- enginetest/queries/query_plans.go | 57 ++++++++++++++++++++----------- 1 file changed, 37 insertions(+), 20 deletions(-) diff --git a/enginetest/queries/query_plans.go b/enginetest/queries/query_plans.go index 4e6dc2b356..91cab5ffea 100644 --- a/enginetest/queries/query_plans.go +++ b/enginetest/queries/query_plans.go @@ -3610,28 +3610,45 @@ Select * from ( "", }, { - Query: `SELECT * FROM (select i,s FROM mytable) mt order by i;`, - ExpectedPlan: "TableAlias(mt)\n" + - " └─ IndexedTableAccess(mytable)\n" + - " ├─ index: [mytable.i]\n" + - " ├─ static: [{[NULL, ∞)}]\n" + - " ├─ colSet: (1,2)\n" + - " ├─ tableId: 1\n" + - " └─ Table\n" + - " ├─ name: mytable\n" + - " └─ columns: [i s]\n" + + Query: `SELECT * FROM (select t.i,t.s FROM mytable t) mt order by i;`, + ExpectedPlan: "SubqueryAlias\n" + + " ├─ name: mt\n" + + " ├─ outerVisibility: false\n" + + " ├─ isLateral: false\n" + + " ├─ cacheable: true\n" + + " ├─ colSet: (3,4)\n" + + " ├─ tableId: 2\n" + + " └─ TableAlias(t)\n" + + " └─ IndexedTableAccess(mytable)\n" + + " ├─ index: [mytable.i]\n" + + " ├─ static: [{[NULL, ∞)}]\n" + + " ├─ colSet: (1,2)\n" + + " ├─ tableId: 1\n" + + " └─ Table\n" + + " ├─ name: mytable\n" + + " └─ columns: [i s]\n" + "", - ExpectedEstimates: "TableAlias(mt)\n" + - " └─ IndexedTableAccess(mytable)\n" + - " ├─ index: [mytable.i]\n" + - " ├─ filters: [{[NULL, ∞)}]\n" + - " └─ columns: [i s]\n" + + ExpectedEstimates: "SubqueryAlias\n" + + " ├─ name: mt\n" + + " ├─ outerVisibility: false\n" + + " ├─ isLateral: false\n" + + " ├─ cacheable: true\n" + + " └─ TableAlias(t)\n" + + " └─ IndexedTableAccess(mytable)\n" + + " ├─ index: [mytable.i]\n" + + " ├─ filters: [{[NULL, ∞)}]\n" + + " └─ columns: [i s]\n" + "", - ExpectedAnalysis: "TableAlias(mt)\n" + - " └─ IndexedTableAccess(mytable)\n" + - " ├─ index: [mytable.i]\n" + - " ├─ filters: [{[NULL, ∞)}]\n" + - " └─ columns: [i s]\n" + + ExpectedAnalysis: "SubqueryAlias\n" + + " ├─ name: mt\n" + + " ├─ outerVisibility: false\n" + + " ├─ isLateral: false\n" + + " ├─ cacheable: true\n" + + " └─ TableAlias(t)\n" + + " └─ IndexedTableAccess(mytable)\n" + + " ├─ index: [mytable.i]\n" + + " ├─ filters: [{[NULL, ∞)}]\n" + + " └─ columns: [i s]\n" + "", }, {