Merge pull request #3131 from dolthub/angela/cte

Unwrap ParenExpr when evaluating OrderBy expressions

Author: angelamayxie

enginetest/queries/order_by_group_by_queries.go

@@ -346,4 +346,26 @@ var OrderByGroupByScriptTests = []ScriptTest{
 			},
 		},
 	},
+	{
+		// https://github.com/dolthub/dolt/issues/9605
+		Name: "Order by wrapped by parentheses",
+		SetUpScript: []string{
+			"create table t(i int, j int)",
+			"insert into t values(2,4),(0,7),(9,10),(4,3)",
+		},
+		Assertions: []ScriptTestAssertion{
+			{
+				Query:    "with cte(i) as (select i from t) select * from cte order by (i)",
+				Expected: []sql.Row{{0}, {2}, {4}, {9}},
+			},
+			{
+				Query:    "with cte(i) as (select i from t) select * from cte order by (((i)))",
+				Expected: []sql.Row{{0}, {2}, {4}, {9}},
+			},
+			{
+				Query:    "select * from t order by (i * 10 + j)",
+				Expected: []sql.Row{{0, 7}, {2, 4}, {4, 3}, {9, 10}},
+			},
+		},
+	},
 }

sql/planbuilder/orderby.go

@@ -46,8 +46,8 @@ func (b *Builder) analyzeOrderBy(fromScope, projScope *scope, order ast.OrderBy)
 		case ast.DescScr:
 			descending = true
 		}
-
-		switch e := o.Expr.(type) {
+		expr := unwrapExpression(o.Expr)
+		switch e := expr.(type) {
 		case *ast.ColName:
 			// check for projection alias first
 			dbName := strings.ToLower(e.Qualifier.DbQualifier.String())
@@ -147,7 +147,7 @@ func (b *Builder) analyzeOrderBy(fromScope, projScope *scope, order ast.OrderBy)
 					// has to have been ref'd already
 					id, ok := fromScope.getExpr(e.String(), true)
 					if !ok {
-						err := fmt.Errorf("faild to ref aggregate expression: %s", e.String())
+						err := fmt.Errorf("failed to ref aggregate expression: %s", e.String())
 						b.handleErr(err)
 					}
 					return expression.NewGetField(int(id), e.Type(), e.String(), e.IsNullable()), transform.NewTree, nil
@@ -226,3 +226,13 @@ func (b *Builder) buildOrderBy(inScope, orderByScope *scope) {
 	inScope.node = sort
 	return
 }
+
+// unwrapExpression unwraps expressions wrapped in ParenExpr (parenthesis)
+// TODO: consider moving this function to a different file or package since it seems like it could be used in other
+// places
+func unwrapExpression(expr ast.Expr) ast.Expr {
+	if parensExpr, ok := expr.(*ast.ParenExpr); ok {
+		return unwrapExpression(parensExpr.Expr)
+	}
+	return expr
+}