Skip to content

Commit bf332d9

Browse files
committed
unwrap multiple parenthesis, simplified tests
1 parent 181d5a5 commit bf332d9

File tree

2 files changed

+24
-42
lines changed

2 files changed

+24
-42
lines changed

enginetest/queries/order_by_group_by_queries.go

Lines changed: 13 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -348,47 +348,23 @@ var OrderByGroupByScriptTests = []ScriptTest{
348348
},
349349
{
350350
// https://github.com/dolthub/dolt/issues/9605
351-
Name: "Order by CTE column wrapped by parentheses",
351+
Name: "Order by wrapped by parentheses",
352352
SetUpScript: []string{
353-
"create table tree_data (id int not null, parent_id int, primary key (id), constraint foreign key (parent_id) references tree_data (id));",
354-
"insert into tree_data values (1,null),(2,null),(3,null),(4,1),(5,4),(6,3)",
353+
"create table t(i int, j int)",
354+
"insert into t values(2,4),(0,7),(9,10),(4,3)",
355355
},
356356
Assertions: []ScriptTestAssertion{
357357
{
358-
Query: `WITH RECURSIVE __rank_table(id, parent_id, rank_order) AS (SELECT tree_data.id,
359-
tree_data.parent_id,
360-
row_number() OVER (
361-
ORDER BY tree_data.id) AS rank_order
362-
FROM tree_data),
363-
__tree(tree_depth, tree_path, tree_ordering, tree_pk) AS
364-
(SELECT 0,
365-
cast(concat("", id, "") AS char(1000)),
366-
cast(concat("", lpad(concat(T.rank_order, ""), 20, "0")) AS char(1000)),
367-
T.id
368-
FROM __rank_table T
369-
WHERE T.parent_id IS NULL
370-
UNION ALL SELECT __tree.tree_depth + 1,
371-
concat(__tree.tree_path, T2.id, ""),
372-
concat(__tree.tree_ordering, lpad(concat(T2.rank_order, ""), 20, "0")),
373-
T2.id
374-
FROM __tree,
375-
__rank_table T2
376-
WHERE __tree.tree_pk = T2.parent_id)
377-
SELECT __tree.tree_depth AS tree_depth,
378-
tree_data.id,
379-
tree_data.parent_id
380-
FROM __tree,
381-
tree_data
382-
WHERE __tree.tree_pk = tree_data.id
383-
ORDER BY (__tree.tree_depth) DESC;`,
384-
Expected: []sql.Row{
385-
{2, 5, 4},
386-
{1, 4, 1},
387-
{1, 6, 3},
388-
{0, 1, nil},
389-
{0, 2, nil},
390-
{0, 3, nil},
391-
},
358+
Query: "with cte(i) as (select i from t) select * from cte order by (i)",
359+
Expected: []sql.Row{{0}, {2}, {4}, {9}},
360+
},
361+
{
362+
Query: "with cte(i) as (select i from t) select * from cte order by (((i)))",
363+
Expected: []sql.Row{{0}, {2}, {4}, {9}},
364+
},
365+
{
366+
Query: "select * from t order by (i * 10 + j)",
367+
Expected: []sql.Row{{0, 7}, {2, 4}, {4, 3}, {9, 10}},
392368
},
393369
},
394370
},

sql/planbuilder/orderby.go

Lines changed: 11 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -46,11 +46,7 @@ func (b *Builder) analyzeOrderBy(fromScope, projScope *scope, order ast.OrderBy)
4646
case ast.DescScr:
4747
descending = true
4848
}
49-
expr := o.Expr
50-
// unwrap expressions wrapped by parentheses
51-
if parensExpr, ok := expr.(*ast.ParenExpr); ok {
52-
expr = parensExpr.Expr
53-
}
49+
expr := unwrapExpression(o.Expr)
5450
switch e := expr.(type) {
5551
case *ast.ColName:
5652
// check for projection alias first
@@ -230,3 +226,13 @@ func (b *Builder) buildOrderBy(inScope, orderByScope *scope) {
230226
inScope.node = sort
231227
return
232228
}
229+
230+
// unwrapExpression unwraps expressions wrapped in ParenExpr (parenthesis)
231+
// TODO: consider moving this function to a different file or package since it seems like it could be used in other
232+
// places
233+
func unwrapExpression(expr ast.Expr) ast.Expr {
234+
if parensExpr, ok := expr.(*ast.ParenExpr); ok {
235+
return unwrapExpression(parensExpr.Expr)
236+
}
237+
return expr
238+
}

0 commit comments

Comments
 (0)