fix(compiler): correctly validate alias in order/group by clauses for joins (#2537)

* fix(compiler): correctly validate alias in order/group by clauses for joins

Resolves #1886
Resolves #2398
Resolves #2399

* remove dead code and split up test

Author: andrewmbenton

internal/compiler/output_columns.go

@@ -69,7 +69,7 @@ func (c *Compiler) outputColumns(qc *QueryCatalog, node ast.Node) ([]*Column, er
 
 		if n.GroupClause != nil {
 			for _, item := range n.GroupClause.Items {
-				if err := findColumnForNode(item, tables, n); err != nil {
+				if err := findColumnForNode(item, tables, targets); err != nil {
 					return nil, err
 				}
 			}
@@ -85,7 +85,7 @@ func (c *Compiler) outputColumns(qc *QueryCatalog, node ast.Node) ([]*Column, er
 					if !ok {
 						continue
 					}
-					if err := findColumnForNode(sb.Node, tables, n); err != nil {
+					if err := findColumnForNode(sb.Node, tables, targets); err != nil {
 						return nil, fmt.Errorf("%v: if you want to skip this validation, set 'strict_order_by' to false", err)
 					}
 				}
@@ -101,7 +101,7 @@ func (c *Compiler) outputColumns(qc *QueryCatalog, node ast.Node) ([]*Column, er
 						if !ok {
 							continue
 						}
-						if err := findColumnForNode(caseExpr.Xpr, tables, n); err != nil {
+						if err := findColumnForNode(caseExpr.Xpr, tables, targets); err != nil {
 							return nil, fmt.Errorf("%v: if you want to skip this validation, set 'strict_order_by' to false", err)
 						}
 					}
@@ -650,15 +650,15 @@ func outputColumnRefs(res *ast.ResTarget, tables []*Table, node *ast.ColumnRef)
 	return cols, nil
 }
 
-func findColumnForNode(item ast.Node, tables []*Table, n *ast.SelectStmt) error {
+func findColumnForNode(item ast.Node, tables []*Table, targetList *ast.List) error {
 	ref, ok := item.(*ast.ColumnRef)
 	if !ok {
 		return nil
 	}
-	return findColumnForRef(ref, tables, n)
+	return findColumnForRef(ref, tables, targetList)
 }
 
-func findColumnForRef(ref *ast.ColumnRef, tables []*Table, selectStatement *ast.SelectStmt) error {
+func findColumnForRef(ref *ast.ColumnRef, tables []*Table, targetList *ast.List) error {
 	parts := stringSlice(ref.Fields)
 	var alias, name string
 	if len(parts) == 1 {
@@ -675,20 +675,17 @@ func findColumnForRef(ref *ast.ColumnRef, tables []*Table, selectStatement *ast.
 		}
 
 		// Find matching column
-		var foundColumn bool
 		for _, c := range t.Columns {
 			if c.Name == name {
 				found++
-				foundColumn = true
+				break
 			}
 		}
+	}
 
-		if foundColumn {
-			continue
-		}
-
-		// Find matching alias
-		for _, c := range selectStatement.TargetList.Items {
+	// Find matching alias if necessary
+	if found == 0 {
+		for _, c := range targetList.Items {
 			resTarget, ok := c.(*ast.ResTarget)
 			if !ok {
 				continue

internal/endtoend/testdata/join_group_by_alias/postgresql/stdlib/go/db.go

@@ -0,0 +1,6 @@
+CREATE TABLE foo (email text not null);
+
+-- name: ColumnAsGroupBy :many
+SELECT a.email AS id
+FROM foo a JOIN foo b ON a.email = b.email
+GROUP BY id;

internal/endtoend/testdata/join_group_by_alias/postgresql/stdlib/go/models.go

@@ -0,0 +1,12 @@
+{
+  "version": "1",
+  "packages": [
+    {
+      "engine": "postgresql",
+      "path": "go",
+      "name": "querytest",
+      "schema": "query.sql",
+      "queries": "query.sql"
+    }
+  ]
+}

internal/endtoend/testdata/join_group_by_alias/postgresql/stdlib/go/query.sql.go

@@ -0,0 +1,6 @@
+CREATE TABLE foo (email text not null);
+
+-- name: ColumnAsOrderBy :many
+SELECT a.email AS id
+FROM foo a JOIN foo b ON a.email = b.email
+ORDER BY id;

internal/endtoend/testdata/join_group_by_alias/postgresql/stdlib/query.sql

@@ -0,0 +1,12 @@
+{
+  "version": "1",
+  "packages": [
+    {
+      "engine": "postgresql",
+      "path": "go",
+      "name": "querytest",
+      "schema": "query.sql",
+      "queries": "query.sql"
+    }
+  ]
+}