Having aggregate alias bug (#2174)

* fix having aggregate resolve bug

* fix spacing

* Add additional tests for resolving `HAVING` expressions.

* Add additional tests for resolving `HAVING` expressions, including cases where we *should* resolve to an alias.

* Fix expected output of test.

* modify tests

* remove comments

---------

Co-authored-by: Nick Tobey <[email protected]>

enginetest/queries/queries.go

@@ -764,6 +764,21 @@ var SpatialQueryTests = []QueryTest{
 }
 
 var QueryTests = []QueryTest{
+	{
+		Query:    "select 1 as x from xy having AVG(x) > 0",
+		Expected: []sql.Row{{1}},
+	}, {
+		Query:    "select 1 as x, AVG(x) from xy group by (y) having AVG(x) > 0",
+		Expected: []sql.Row{{1, float64(1)}, {1, float64(2)}, {1, float64(3)}},
+	},
+	{
+		Query:    "select y as x from xy group by (y) having AVG(x) > 0",
+		Expected: []sql.Row{{0}, {1}, {3}},
+	},
+	//{
+	//	Query:    "select y as z from xy group by (y) having AVG(z) > 0",
+	//	Expected: []sql.Row{{1}, {2}, {3}},
+	//},
 	{
 		Query:    "select x from xy where y in (select xy.x from xy join (select t2.y from xy t2 where exists (select t3.y from xy t3 where t3.y = xy.x)) t1) order by 1;",
 		Expected: []sql.Row{{0}, {1}, {2}, {3}},
@@ -3468,19 +3483,19 @@ Select * from (
 	},
 	{
 		Query:    `SELECT TRIM(mytable.s) AS s FROM mytable`,
-		Expected: []sql.Row{sql.Row{"first row"}, sql.Row{"second row"}, sql.Row{"third row"}},
+		Expected: []sql.Row{{"first row"}, {"second row"}, {"third row"}},
 	},
 	{
 		Query:    `SELECT TRIM("row" from mytable.s) AS s FROM mytable`,
-		Expected: []sql.Row{sql.Row{"first "}, sql.Row{"second "}, sql.Row{"third "}},
+		Expected: []sql.Row{{"first "}, {"second "}, {"third "}},
 	},
 	{
 		Query:    `SELECT TRIM(mytable.s from "first row") AS s FROM mytable`,
-		Expected: []sql.Row{sql.Row{""}, sql.Row{"first row"}, sql.Row{"first row"}},
+		Expected: []sql.Row{{""}, {"first row"}, {"first row"}},
 	},
 	{
 		Query:    `SELECT TRIM(mytable.s from mytable.s) AS s FROM mytable`,
-		Expected: []sql.Row{sql.Row{""}, sql.Row{""}, sql.Row{""}},
+		Expected: []sql.Row{{""}, {""}, {""}},
 	},
 	{
 		Query:    `SELECT TRIM("   foo   ")`,

sql/analyzer/analyzer.go

@@ -487,8 +487,6 @@ func newInsertSourceSelector(sel RuleSelector) RuleSelector {
 // Analyze applies the transformation rules to the node given. In the case of an error, the last successfully
 // transformed node is returned along with the error.
 func (a *Analyzer) Analyze(ctx *sql.Context, n sql.Node, scope *plan.Scope) (sql.Node, error) {
-	//a.Verbose = true
-	//a.Debug = true
 	n, _, err := a.analyzeWithSelector(ctx, n, scope, SelectAllBatches, DefaultRuleSelector)
 	return n, err
 }

sql/planbuilder/aggregates.go

@@ -310,6 +310,15 @@ func (b *Builder) buildAggregateFunc(inScope *scope, name string, e *ast.FuncExp
 		e := b.selectExprToExpression(inScope, arg)
 		switch e := e.(type) {
 		case *expression.GetField:
+			if e.TableId() == 0 {
+				// aliases are not valid aggregate arguments, the alias must be masking a column
+				gf := b.selectExprToExpression(inScope.parent, arg)
+				var ok bool
+				e, ok = gf.(*expression.GetField)
+				if !ok || e.TableId() == 0 {
+					b.handleErr(fmt.Errorf("failed to resolve aggregate column argument: %s", gf))
+				}
+			}
 			args = append(args, e)
 			col := scopeColumn{tableId: e.TableID(), db: e.Database(), table: e.Table(), col: e.Name(), scalar: e, typ: e.Type(), nullable: e.IsNullable()}
 			gb.addInCol(col)

sql/planbuilder/parse_test.go

@@ -48,9 +48,81 @@ type planErrTest struct {
 func TestPlanBuilder(t *testing.T) {
 	var verbose, rewrite bool
 	//verbose = true
-	rewrite = true
+	//rewrite = true
 
 	var tests = []planTest{
+		{
+			Query: "with cte(x) as (select 1 as x) select 1 as x from cte having avg(x) > 0",
+			ExpectedPlan: `
+Project
+ ├─ columns: [1 (tinyint) as x]
+ └─ Having
+     ├─ GreaterThan
+     │   ├─ avg(cte.x):4
+     │   └─ 0 (tinyint)
+     └─ Project
+         ├─ columns: [avg(cte.x):4, 1 (tinyint) as x]
+         └─ GroupBy
+             ├─ select: AVG(cte.x:2!null)
+             ├─ group: 
+             └─ SubqueryAlias
+                 ├─ name: cte
+                 ├─ outerVisibility: false
+                 ├─ isLateral: false
+                 ├─ cacheable: true
+                 ├─ colSet: (2)
+                 ├─ tableId: 1
+                 └─ Project
+                     ├─ columns: [1 (tinyint) as x]
+                     └─ Table
+                         ├─ name: 
+                         ├─ columns: []
+                         ├─ colSet: ()
+                         └─ tableId: 0
+`,
+		},
+		{
+			Query: "select 1 as x from xy having AVG(x) > 0",
+			ExpectedPlan: `
+Project
+ ├─ columns: [1 (tinyint) as x]
+ └─ Having
+     ├─ GreaterThan
+     │   ├─ avg(xy.x):5
+     │   └─ 0 (tinyint)
+     └─ Project
+         ├─ columns: [avg(xy.x):5, 1 (tinyint) as x]
+         └─ GroupBy
+             ├─ select: AVG(xy.x:1!null)
+             ├─ group: 
+             └─ Table
+                 ├─ name: xy
+                 ├─ columns: [x y z]
+                 ├─ colSet: (1-3)
+                 └─ tableId: 1
+`,
+		},
+		{
+			Query: "select x as x from xy having avg(x) > 0",
+			ExpectedPlan: `
+Project
+ ├─ columns: [xy.x:1!null as x]
+ └─ Having
+     ├─ GreaterThan
+     │   ├─ avg(xy.x):5
+     │   └─ 0 (tinyint)
+     └─ Project
+         ├─ columns: [avg(xy.x):5, xy.x:1!null, xy.x:1!null as x]
+         └─ GroupBy
+             ├─ select: AVG(xy.x:1!null), xy.x:1!null
+             ├─ group: 
+             └─ Table
+                 ├─ name: xy
+                 ├─ columns: [x y z]
+                 ├─ colSet: (1-3)
+                 └─ tableId: 1
+`,
+		},
 		{
 			Query: "select x, x from xy order by x",
 			ExpectedPlan: `