Repository: vitessio/vitess
Pull request #8683 (2021-08-25 17:11:00)

Gen4: Having clause support
<!--
  Thank you for your contribution to the Vitess project.
  How to contribute: https://vitess.io/docs/contributing/
  Please first make sure there is an open Issue to discuss the feature/fix suggested in this PR.
  If this is a new feature, please mark the Issue as "RFC".
 -->

<!-- if this PR is Work in Progress please create it as a Draft Pull Request -->

## Description
<!-- A few sentences describing the overall goals of the pull request's commits. -->
<!-- If this is a bug fix and you think the fix should be backported, please write so. -->

Add support for having clause in Gen4 plan.

## Related Issue(s)
<!-- List related issues and pull requests. If this PR fixes an issue, please add it using Fixes #????  -->

- #7280 
- Closes https://github.com/vitessio/vitess/issues/8576

## Checklist
- [X] Tests were added or are not required
- [X] Documentation was added or is not required

## Deployment Notes
<!-- Notes regarding deployment of the contained body of work. These should note any db migrations, etc. -->


Patch:
diff --git a/go/vt/vtgate/planbuilder/gen4_planner.go b/go/vt/vtgate/planbuilder/gen4_planner.go
index 09966c8e21..146cbb7d0c 100644
--- a/go/vt/vtgate/planbuilder/gen4_planner.go
+++ b/go/vt/vtgate/planbuilder/gen4_planner.go
@@ -74,7 +74,7 @@ func newBuildSelectPlan(sel *sqlparser.Select, reservedVars *sqlparser.ReservedV
 	}
 
 	ctx := newPlanningContext(reservedVars, semTable, vschema)
-	err = subqueryRewrite(ctx, sel)
+	err = queryRewrite(ctx, sel)
 	if err != nil {
 		return nil, err
 	}
@@ -155,8 +155,8 @@ func planLimit(limit *sqlparser.Limit, plan logicalPlan) (logicalPlan, error) {
 }
 
 func checkUnsupportedConstructs(sel *sqlparser.Select) error {
-	if sel.Having != nil {
-		return semantics.Gen4NotSupportedF("HAVING")
+	if sel.SQLCalcFoundRows {
+		return semantics.Gen4NotSupportedF("sql_calc_found_rows")
 	}
 	return nil
 }
diff --git a/go/vt/vtgate/planbuilder/horizon_planning.go b/go/vt/vtgate/planbuilder/horizon_planning.go
index e2280be41e..573bc9f0e0 100644
--- a/go/vt/vtgate/planbuilder/horizon_planning.go
+++ b/go/vt/vtgate/planbuilder/horizon_planning.go
@@ -60,7 +60,7 @@ func (hp *horizonPlanning) planHorizon(ctx planningContext, plan logicalPlan) (l
 		return nil, err
 	}
 
-	if hp.qp.NeedsAggregation() {
+	if hp.qp.NeedsAggregation() || hp.sel.Having != nil {
 		plan, err = hp.planAggregations(ctx, plan)
 		if err != nil {
 			return nil, err
@@ -329,6 +329,11 @@ func (hp *horizonPlanning) planAggregations(ctx planningContext, plan logicalPla
 		hp.haveToTruncate(added)
 	}
 
+	err := hp.planHaving(ctx, newPlan)
+	if err != nil {
+		return nil, err
+	}
+
 	if !hp.qp.CanPushDownSorting && oa != nil {
 		var orderExprs []abstract.OrderBy
 		// if we can't at a later stage push down the sorting to our inputs, we have to do ordering here
@@ -837,3 +842,37 @@ func (hp *horizonPlanning) needDistinctHandling(ctx planningContext, funcExpr *s
 	}
 	return true, innerAliased, nil
 }
+
+func (hp *horizonPlanning) planHaving(ctx planningContext, plan logicalPlan) error {
+	if hp.sel.Having == nil {
+		return nil
+	}
+	for _, expr := range sqlparser.SplitAndExpression(nil, hp.sel.Having.Expr) {
+		err := pushHaving(expr, plan, ctx.semTable)
+		if err != nil {
+			return err
+		}
+	}
+	return nil
+}
+
+func pushHaving(expr sqlparser.Expr, plan logicalPlan, semTable *semantics.SemTable) error {
+	switch node := plan.(type) {
+	case *route:
+		sel, ok := node.Select.(*sqlparser.Select)
+		if !ok {
+			return vterrors.Errorf(vtrpcpb.Code_UNIMPLEMENTED, "unsupported: filtering on unexpected select statement: %T", node.Select)
+		}
+		sel.AddHaving(expr)
+		return nil
+	case *join:
+		return semantics.Gen4NotSupportedF("having on join")
+	case *pulloutSubquery:
+		return pushHaving(expr, node.underlying, semTable)
+	case *simpleProjection:
+		return vterrors.Errorf(vtrpcpb.Code_UNIMPLEMENTED, "unsupported: filtering on results of cross-shard derived table")
+	case *orderedAggregate:
+		return vterrors.Errorf(vtrpcpb.Code_UNIMPLEMENTED, "unsupported: filtering on results of aggregates")
+	}
+	return vterrors.Errorf(vtrpcpb.Code_INTERNAL, "[BUG] unreachable %T.filtering", plan)
+}
diff --git a/go/vt/vtgate/planbuilder/rewrite.go b/go/vt/vtgate/planbuilder/rewrite.go
index 8ae13429c6..e784568559 100644
--- a/go/vt/vtgate/planbuilder/rewrite.go
+++ b/go/vt/vtgate/planbuilder/rewrite.go
@@ -24,13 +24,11 @@ import (
 	"vitess.io/vitess/go/vt/vtgate/semantics"
 )
 
-type (
-	rewriter struct {
-		err          error
-		semTable     *semantics.SemTable
-		reservedVars *sqlparser.ReservedVars
-	}
-)
+type rewriter struct {
+	err          error
+	semTable     *semantics.SemTable
+	reservedVars *sqlparser.ReservedVars
+}
 
 func starRewrite(statement sqlparser.SelectStatement, semTable *semantics.SemTable) error {
 	r := rewriter{
@@ -109,48 +107,96 @@ func expandTableColumns(tables []semantics.TableInfo, starExpr *sqlparser.StarEx
 	return starExpanded, colNames, nil
 }
 
-func subqueryRewrite(ctx planningContext, statement sqlparser.SelectStatement) error {
-	if len(ctx.semTable.SubqueryMap) == 0 {
-		return nil
-	}
+func queryRewrite(ctx planningContext, statement sqlparser.SelectStatement) error {
 	r := rewriter{
 		semTable:     ctx.semTable,
 		reservedVars: ctx.reservedVars,
 	}
-	sqlparser.Rewrite(statement, r.subqueryRewrite, nil)
+	sqlparser.Rewrite(statement, r.rewrite, nil)
 	return nil
 }
 
-func (r *rewriter) subqueryRewrite(cursor *sqlparser.Cursor) bool {
+func (r *rewriter) rewrite(cursor *sqlparser.Cursor) bool {
 	switch node := cursor.Node().(type) {
+	case *sqlparser.Select:
+		rewriteHavingClause(node)
 	case *sqlparser.ExistsExpr:
-		semTableSQ, found := r.semTable.SubqueryRef[node.Subquery]
-		if !found {
-			// should never happen
-			return false
-		}
+		return r.rewriteExistsSubquery(cursor, node)
+	case *sqlparser.Subquery:
+		rewriteSubquery(cursor, r, node)
+	}
+	return true
+}
 
-		argName := r.reservedVars.ReserveHasValuesSubQuery()
-		semTableSQ.ArgName = argName
+func rewriteSubquery(cursor *sqlparser.Cursor, r *rewriter, node *sqlparser.Subquery) {
+	semTableSQ, found := r.semTable.SubqueryRef[node]
+	if !found {
+		// should never happen
+		return
+	}
 
+	argName := r.reservedVars.ReserveSubQuery()
+	semTableSQ.ArgName = argName
+
+	switch semTableSQ.OpCode {
+	case engine.PulloutIn, engine.PulloutNotIn:
+		cursor.Replace(sqlparser.NewListArg(argName))
+	default:
 		cursor.Replace(sqlparser.NewArgument(argName))
+	}
+}
+
+func (r *rewriter) rewriteExistsSubquery(cursor *sqlparser.Cursor, node *sqlparser.ExistsExpr) bool {
+	semTableSQ, found := r.semTable.SubqueryRef[node.Subquery]
+	if !found {
+		// should never happen
 		return false
-	case *sqlparser.Subquery:
-		semTableSQ, found := r.semTable.SubqueryRef[node]
-		if !found {
-			// should never happen
-			return false
-		}
+	}
 
-		argName := r.reservedVars.ReserveSubQuery()
-		semTableSQ.ArgName = argName
+	argName := r.reservedVars.ReserveHasValuesSubQuery()
+	semTableSQ.ArgName = argName
 
-		switch semTableSQ.OpCode {
-		case engine.PulloutIn, engine.PulloutNotIn:
-			cursor.Replace(sqlparser.NewListArg(argName))
-		default:
-			cursor.Replace(sqlparser.NewArgument(argName))
+	cursor.Replace(sqlparser.NewArgument(argName))
+	return false
+}
+
+func rewriteHavingClause(node *sqlparser.Select) {
+	if node.Having == nil {
+		return
+	}
+
+	selectExprMap := map[string]sqlparser.Expr{}
+	for _, selectExpr := range node.SelectExprs {
+		aliasedExpr, isAliased := selectExpr.(*sqlparser.AliasedExpr)
+		if !isAliased || aliasedExpr.As.IsEmpty() {
+			continue
+		}
+		selectExprMap[aliasedExpr.As.Lowered()] = aliasedExpr.Expr
+	}
+
+	sqlparser.Rewrite(node.Having.Expr, func(cursor *sqlparser.Cursor) bool {
+		switch x := cursor.Node().(type) {
+		case *sqlparser.ColName:
+			if !x.Qualifier.IsEmpty() {
+				return false
+			}
+			originalExpr, isInMap := selectExprMap[x.Name.Lowered()]
+			if isInMap {
+				cursor.Replace(originalExpr)
+				return false
+			}
+			return false
+		}
+		return true
+	}, nil)
+
+	exprs := sqlparser.SplitAndExpression(nil, node.Having.Expr)
+	node.Having = nil
+	for _, expr := range exprs {
+		if sqlparser.ContainsAggregation(expr) {
+			node.AddHaving(expr)
+		} else {
+			node.AddWhere(expr)
 		}
 	}
-	return true
 }
diff --git a/go/vt/vtgate/planbuilder/route_planning.go b/go/vt/vtgate/planbuilder/route_planning.go
index ceabc57da7..7f6d4f60c5 100644
--- a/go/vt/vtgate/planbuilder/route_planning.go
+++ b/go/vt/vtgate/planbuilder/route_planning.go
@@ -194,6 +194,7 @@ func createSingleShardRoutePlan(sel *sqlparser.Select, rb *route) {
 	ast := rb.Select.(*sqlparser.Select)
 	ast.Distinct = sel.Distinct
 	ast.GroupBy = sel.GroupBy
+	ast.Having = sel.Having
 	ast.OrderBy = sel.OrderBy
 	ast.Comments = sel.Comments
 	ast.SelectExprs = sel.SelectExprs
diff --git a/go/vt/vtgate/semantics/analyzer.go b/go/vt/vtgate/semantics/analyzer.go
index d2a8092b5e..cfe1b16b1d 100644
--- a/go/vt/vtgate/semantics/analyzer.go
+++ b/go/vt/vtgate/semantics/analyzer.go
@@ -246,10 +246,6 @@ func checkForInvalidConstructs(cursor *sqlparser.Cursor) error {
 		if node.Join == sqlparser.NaturalJoinType || node.Join == sqlparser.NaturalRightJoinType || node.Join == sqlparser.NaturalLeftJoinType {
 			return vterrors.New(vtrpcpb.Code_UNIMPLEMENTED, "unsupported: "+node.Join.ToString())
 		}
-	case *sqlparser.Select:
-		if node.Having != nil {
-			return Gen4NotSupportedF("HAVING")
-		}
 	case *sqlparser.Subquery:
 		sel, ok := node.Select.(*sqlparser.Select)
 		if !ok {
diff --git a/go/vt/vtgate/semantics/binder.go b/go/vt/vtgate/semantics/binder.go
index 949de38725..05b17c554e 100644
--- a/go/vt/vtgate/semantics/binder.go
+++ b/go/vt/vtgate/semantics/binder.go
@@ -112,6 +112,9 @@ func (b *binder) down(cursor *sqlparser.Cursor) error {
 		scope := b.scoper.currentScope()
 		ts := TableSet(0)
 		for _, table := range scope.tables {
+			if !table.IsActualTable() {
+				continue
+			}
 			ts |= b.tc.tableSetFor(table.GetExpr())
 		}
 		b.exprRecursiveDeps[node] = ts
diff --git a/go/vt/vtgate/semantics/scoper.go b/go/vt/vtgate/semantics/scoper.go
index 17d34b6bd7..ff5069143e 100644
--- a/go/vt/vtgate/semantics/scoper.go
+++ b/go/vt/vtgate/semantics/scoper.go
@@ -32,11 +32,19 @@ type scoper struct {
 }
 
 type scopeKey struct {
-	orderBy bool
-	groupBy bool
-	node    sqlparser.SQLNode
+	typ  keyType
+	node sqlparser.SQLNode
 }
 
+type keyType int8
+
+const (
+	_ keyType = iota
+	orderBy
+	groupBy
+	having
+)
+
 func newScoper() *scoper {
 	return &scoper{
 		rScope:       map[*sqlparser.Select]*scope{},
@@ -85,16 +93,26 @@ func (s *scoper) down(cursor *sqlparser.Cursor) {
 
 		wScope.tables = append(wScope.tables, createVTableInfoForExpressions(node))
 	case sqlparser.OrderBy:
-		s.changeScopeForOrderBy(cursor, scopeKey{node: cursor.Parent(), orderBy: true})
+		s.changeScopeForNode(cursor, scopeKey{node: cursor.Parent(), typ: orderBy})
 	case sqlparser.GroupBy:
-		s.changeScopeForOrderBy(cursor, scopeKey{node: cursor.Parent(), groupBy: true})
+		s.changeScopeForNode(cursor, scopeKey{node: cursor.Parent(), typ: groupBy})
+	case *sqlparser.Where:
+		if node.Type != sqlparser.HavingClause {
+			break
+		}
+		s.changeScopeForNode(cursor, scopeKey{node: cursor.Parent(), typ: having})
 	}
 }
 
 func (s *scoper) up(cursor *sqlparser.Cursor) error {
-	switch cursor.Node().(type) {
+	switch node := cursor.Node().(type) {
 	case sqlparser.SelectStatement, sqlparser.OrderBy, sqlparser.GroupBy:
 		s.popScope()
+	case *sqlparser.Where:
+		if node.Type != sqlparser.HavingClause {
+			break
+		}
+		s.popScope()
 	case sqlparser.TableExpr:
 		if isParentSelect(cursor) {
 			curScope := s.currentScope()
@@ -118,9 +136,14 @@ func (s *scoper) downPost(cursor *sqlparser.Cursor) {
 
 	switch node := cursor.Node().(type) {
 	case sqlparser.OrderBy:
-		scope, found = s.sqlNodeScope[scopeKey{node: cursor.Parent(), orderBy: true}]
+		scope, found = s.sqlNodeScope[scopeKey{node: cursor.Parent(), typ: orderBy}]
 	case sqlparser.GroupBy:
-		scope, found = s.sqlNodeScope[scopeKey{node: cursor.Parent(), groupBy: true}]
+		scope, found = s.sqlNodeScope[scopeKey{node: cursor.Parent(), typ: groupBy}]
+	case *sqlparser.Where:
+		if node.Type != sqlparser.HavingClause {
+			break
+		}
+		scope, found = s.sqlNodeScope[scopeKey{node: cursor.Parent(), typ: having}]
 	default:
 		if validAsMapKey(node) {
 			scope, found = s.sqlNodeScope[scopeKey{node: node}]
@@ -141,9 +164,14 @@ func (s *scoper) upPost(cursor *sqlparser.Cursor) error {
 
 	switch node := cursor.Node().(type) {
 	case sqlparser.OrderBy:
-		_, found = s.sqlNodeScope[scopeKey{node: cursor.Parent(), orderBy: true}]
+		_, found = s.sqlNodeScope[scopeKey{node: cursor.Parent(), typ: orderBy}]
 	case sqlparser.GroupBy:
-		_, found = s.sqlNodeScope[scopeKey{node: cursor.Parent(), groupBy: true}]
+		_, found = s.sqlNodeScope[scopeKey{node: cursor.Parent(), typ: groupBy}]
+	case *sqlparser.Where:
+		if node.Type != sqlparser.HavingClause {
+			break
+		}
+		_, found = s.sqlNodeScope[scopeKey{node: cursor.Parent(), typ: having}]
 	default:
 		if validAsMapKey(node) {
 			_, found = s.sqlNodeScope[scopeKey{node: node}]
@@ -156,12 +184,12 @@ func (s *scoper) upPost(cursor *sqlparser.Cursor) error {
 	return nil
 }
 
-func (s *scoper) changeScopeForOrderBy(cursor *sqlparser.Cursor, k scopeKey) {
+func (s *scoper) changeScopeForNode(cursor *sqlparser.Cursor, k scopeKey) {
 	sel, ok := cursor.Parent().(*sqlparser.Select)
 	if !ok {
 		return
 	}
-	// In ORDER BY, we can see both the scope in the FROM part of the query, and the SELECT columns created
+	// In ORDER BY, GROUP BY and HAVING, we can see both the scope in the FROM part of the query, and the SELECT columns created
 	// so before walking the rest of the tree, we change the scope to match this behaviour
 	incomingScope := s.currentScope()
 	nScope := newScope(incomingScope)
diff --git a/go/vt/vtgate/semantics/semantic_state.go b/go/vt/vtgate/semantics/semantic_state.go
index 306964cc50..244ed368e1 100644
--- a/go/vt/vtgate/semantics/semantic_state.go
+++ b/go/vt/vtgate/semantics/semantic_state.go
@@ -72,7 +72,7 @@ type (
 	}
 
 	// vTableInfo is used to represent projected results, not real tables. It is used for
-	// ORDER BY and GROUP BY that need to access result columns, and also for derived tables.
+	// ORDER BY, GROUP BY and HAVING that need to access result columns, and also for derived tables.
 	vTableInfo struct {
 		tableName   string
 		ASTNode     *sqlparser.AliasedTableExpr
@@ -170,12 +170,25 @@ func (v *vTableInfo) RecursiveDepsFor(col *sqlparser.ColName, org originable, si
 		// if we have a table qualifier in the expression, we know that it is not referencing an aliased table
 		return nil, nil, nil
 	}
+	var tsF TableSet
+	var qtF *querypb.Type
+	found := false
 	for i, colName := range v.columnNames {
 		if col.Name.String() == colName {
 			ts, qt := org.depsForExpr(v.cols[i])
-			return &ts, qt, nil
+			if !found {
+				tsF = ts
+				qtF = qt
+			} else if tsF != ts {
+				// the column does not resolve to the same TableSet. Therefore, it is an ambiguous column reference.
+				return nil, nil, vterrors.NewErrorf(vtrpcpb.Code_INVALID_ARGUMENT, vterrors.NonUniqError, "Column '%s' is ambiguous", colName)
+			}
+			found = true
 		}
 	}
+	if found {
+		return &tsF, qtF, nil
+	}
 	return nil, nil, nil
 }
 


Test patch:
diff --git a/go/vt/vtgate/planbuilder/rewrite_test.go b/go/vt/vtgate/planbuilder/rewrite_test.go
index 51ddf399fb..888447a8b8 100644
--- a/go/vt/vtgate/planbuilder/rewrite_test.go
+++ b/go/vt/vtgate/planbuilder/rewrite_test.go
@@ -230,9 +230,87 @@ func TestSubqueryRewrite(t *testing.T) {
 			semTable, err := semantics.Analyze(selectStatement, "", &semantics.FakeSI{}, semantics.NoRewrite)
 			require.NoError(t, err)
 			ctx := newPlanningContext(reservedVars, semTable, nil)
-			err = subqueryRewrite(ctx, selectStatement)
+			err = queryRewrite(ctx, selectStatement)
 			require.NoError(t, err)
 			assert.Equal(t, tcase.output, sqlparser.String(selectStatement))
 		})
 	}
 }
+
+func TestHavingRewrite(t *testing.T) {
+	tcases := []struct {
+		input  string
+		output string
+		sqs    map[string]string
+	}{{
+		input:  "select 1 from t1 having a = 1",
+		output: "select 1 from t1 where a = 1",
+	}, {
+		input:  "select 1 from t1 where x = 1 and y = 2 having a = 1",
+		output: "select 1 from t1 where x = 1 and y = 2 and a = 1",
+	}, {
+		input:  "select 1 from t1 where x = 1 or y = 2 having a = 1",
+		output: "select 1 from t1 where (x = 1 or y = 2) and a = 1",
+	}, {
+		input:  "select 1 from t1 where x = 1 having a = 1 and b = 2",
+		output: "select 1 from t1 where x = 1 and a = 1 and b = 2",
+	}, {
+		input:  "select 1 from t1 where x = 1 having a = 1 or b = 2",
+		output: "select 1 from t1 where x = 1 and (a = 1 or b = 2)",
+	}, {
+		input:  "select 1 from t1 where x = 1 and y = 2 having a = 1 and b = 2",
+		output: "select 1 from t1 where x = 1 and y = 2 and a = 1 and b = 2",
+	}, {
+		input:  "select 1 from t1 where x = 1 or y = 2 having a = 1 and b = 2",
+		output: "select 1 from t1 where (x = 1 or y = 2) and a = 1 and b = 2",
+	}, {
+		input:  "select 1 from t1 where x = 1 and y = 2 having a = 1 or b = 2",
+		output: "select 1 from t1 where x = 1 and y = 2 and (a = 1 or b = 2)",
+	}, {
+		input:  "select 1 from t1 where x = 1 or y = 2 having a = 1 or b = 2",
+		output: "select 1 from t1 where (x = 1 or y = 2) and (a = 1 or b = 2)",
+	}, {
+		input:  "select 1 from t1 where x = 1 or y = 2 having a = 1 and count(*) = 1",
+		output: "select 1 from t1 where (x = 1 or y = 2) and a = 1 having count(*) = 1",
+	}, {
+		input:  "select count(*) k from t1 where x = 1 or y = 2 having a = 1 and k = 1",
+		output: "select count(*) as k from t1 where (x = 1 or y = 2) and a = 1 having count(*) = 1",
+	}, {
+		input:  "select 1 from t1 where x in (select 1 from t2 having a = 1)",
+		output: "select 1 from t1 where x in ::__sq1",
+		sqs:    map[string]string{"__sq1": "select 1 from t2 where a = 1"},
+	}, {
+		input:  "select 1 from t1 group by a having a = 1 and count(*) > 1",
+		output: "select 1 from t1 where a = 1 group by a having count(*) > 1",
+	}}
+	for _, tcase := range tcases {
+		t.Run(tcase.input, func(t *testing.T) {
+			ctx, sel := prepTest(t, tcase.input)
+			err := queryRewrite(ctx, sel)
+			require.NoError(t, err)
+			assert.Equal(t, tcase.output, sqlparser.String(sel))
+			squeries, found := ctx.semTable.SubqueryMap[sel]
+			if len(tcase.sqs) > 0 {
+				assert.True(t, found, "no subquery found in the query")
+				assert.Equal(t, len(tcase.sqs), len(squeries), "number of subqueries not matched")
+			}
+			for _, sq := range squeries {
+				assert.Equal(t, tcase.sqs[sq.ArgName], sqlparser.String(sq.SubQuery.Select))
+			}
+		})
+	}
+}
+
+func prepTest(t *testing.T, sql string) (planningContext, *sqlparser.Select) {
+	ast, vars, err := sqlparser.Parse2(sql)
+	require.NoError(t, err)
+
+	sel, isSelectStatement := ast.(*sqlparser.Select)
+	require.True(t, isSelectStatement, "analyzer expects a select statement")
+
+	reservedVars := sqlparser.NewReservedVars("vtg", vars)
+	semTable, err := semantics.Analyze(sel, "", &semantics.FakeSI{}, semantics.NoRewrite)
+	require.NoError(t, err)
+
+	return newPlanningContext(reservedVars, semTable, nil), sel
+}
diff --git a/go/vt/vtgate/planbuilder/testdata/aggr_cases.txt b/go/vt/vtgate/planbuilder/testdata/aggr_cases.txt
index f922311e7e..87dcd0edc5 100644
--- a/go/vt/vtgate/planbuilder/testdata/aggr_cases.txt
+++ b/go/vt/vtgate/planbuilder/testdata/aggr_cases.txt
@@ -796,6 +796,44 @@ Gen4 error: Expression of SELECT list is not in GROUP BY clause and contains non
     "Vindex": "user_index"
   }
 }
+{
+  "QueryType": "SELECT",
+  "Original": "select id, count(*) c from user group by id having id=1 and c=10",
+  "Instructions": {
+    "OperatorType": "Route",
+    "Variant": "SelectEqualUnique",
+    "Keyspace": {
+      "Name": "user",
+      "Sharded": true
+    },
+    "FieldQuery": "select id, count(*) as c from `user` where 1 != 1 group by id",
+    "Query": "select id, count(*) as c from `user` where id = 1 group by id having count(*) = 10",
+    "Table": "`user`",
+    "Values": [
+      1
+    ],
+    "Vindex": "user_index"
+  }
+}
+
+# group by a unique vindex should revert to simple route, and having clause should find the correct symbols.
+"select id, count(*) c from user group by id having max(col) > 10"
+{
+  "QueryType": "SELECT",
+  "Original": "select id, count(*) c from user group by id having max(col) \u003e 10",
+  "Instructions": {
+    "OperatorType": "Route",
+    "Variant": "SelectScatter",
+    "Keyspace": {
+      "Name": "user",
+      "Sharded": true
+    },
+    "FieldQuery": "select id, count(*) as c from `user` where 1 != 1 group by id",
+    "Query": "select id, count(*) as c from `user` group by id having max(col) \u003e 10",
+    "Table": "`user`"
+  }
+}
+Gen4 plan same as above
 
 # scatter aggregate in a subquery
 "select a from (select count(*) as a from user) t"
@@ -2471,3 +2509,88 @@ Gen4 plan same as above
     ]
   }
 }
+
+# join query on sharding key with group by a unique vindex with having clause.
+"select user.id, count(*) c from user, user_extra where user.id = user_extra.user_id group by user.id having max(user.col) > 10"
+"unsupported: cross-shard query with aggregates"
+{
+  "QueryType": "SELECT",
+  "Original": "select user.id, count(*) c from user, user_extra where user.id = user_extra.user_id group by user.id having max(user.col) \u003e 10",
+  "Instructions": {
+    "OperatorType": "Route",
+    "Variant": "SelectScatter",
+    "Keyspace": {
+      "Name": "user",
+      "Sharded": true
+    },
+    "FieldQuery": "select `user`.id, count(*) as c from `user`, user_extra where 1 != 1 group by `user`.id",
+    "Query": "select `user`.id, count(*) as c from `user`, user_extra where `user`.id = user_extra.user_id group by `user`.id having max(`user`.col) \u003e 10",
+    "Table": "`user`, user_extra"
+  }
+}
+
+# correlated subquery on sharding key with group by a unique vindex with having clause.
+"select count(*) from user where exists (select 1 from user_extra where user_id = user.id group by user_id having max(col) > 10)"
+{
+  "QueryType": "SELECT",
+  "Original": "select count(*) from user where exists (select 1 from user_extra where user_id = user.id group by user_id having max(col) \u003e 10)",
+  "Instructions": {
+    "OperatorType": "Aggregate",
+    "Variant": "Ordered",
+    "Aggregates": "count(0)",
+    "Inputs": [
+      {
+        "OperatorType": "Route",
+        "Variant": "SelectScatter",
+        "Keyspace": {
+          "Name": "user",
+          "Sharded": true
+        },
+        "FieldQuery": "select count(*) from `user` where 1 != 1",
+        "Query": "select count(*) from `user` where exists (select 1 from user_extra where user_id = `user`.id group by user_id having max(col) \u003e 10)",
+        "Table": "`user`"
+      }
+    ]
+  }
+}
+{
+  "QueryType": "SELECT",
+  "Original": "select count(*) from user where exists (select 1 from user_extra where user_id = user.id group by user_id having max(col) \u003e 10)",
+  "Instructions": {
+    "OperatorType": "Aggregate",
+    "Variant": "Ordered",
+    "Aggregates": "count(0) AS count(*)",
+    "Inputs": [
+      {
+        "OperatorType": "Route",
+        "Variant": "SelectScatter",
+        "Keyspace": {
+          "Name": "user",
+          "Sharded": true
+        },
+        "FieldQuery": "select count(*) from `user` where 1 != 1",
+        "Query": "select count(*) from `user` where exists (select 1 from user_extra where user_id = `user`.id group by user_id having max(col) \u003e 10)",
+        "Table": "`user`"
+      }
+    ]
+  }
+}
+
+# aggregation filtering by having on a route
+"select id from user group by id having count(id) = 10"
+{
+  "QueryType": "SELECT",
+  "Original": "select id from user group by id having count(id) = 10",
+  "Instructions": {
+    "OperatorType": "Route",
+    "Variant": "SelectScatter",
+    "Keyspace": {
+      "Name": "user",
+      "Sharded": true
+    },
+    "FieldQuery": "select id from `user` where 1 != 1 group by id",
+    "Query": "select id from `user` group by id having count(id) = 10",
+    "Table": "`user`"
+  }
+}
+Gen4 plan same as above
diff --git a/go/vt/vtgate/planbuilder/testdata/postprocess_cases.txt b/go/vt/vtgate/planbuilder/testdata/postprocess_cases.txt
index 340f7f63d0..e19a91d502 100644
--- a/go/vt/vtgate/planbuilder/testdata/postprocess_cases.txt
+++ b/go/vt/vtgate/planbuilder/testdata/postprocess_cases.txt
@@ -15,10 +15,26 @@
     "Table": "`user`"
   }
 }
+{
+  "QueryType": "SELECT",
+  "Original": "select user.col1 from user having col2 = 2",
+  "Instructions": {
+    "OperatorType": "Route",
+    "Variant": "SelectScatter",
+    "Keyspace": {
+      "Name": "user",
+      "Sharded": true
+    },
+    "FieldQuery": "select `user`.col1 from `user` where 1 != 1",
+    "Query": "select `user`.col1 from `user` where col2 = 2",
+    "Table": "`user`"
+  }
+}
 
 # ambiguous symbol reference
 "select user.col1, user_extra.col1 from user join user_extra having col1 = 2"
 "ambiguous symbol reference: col1"
+Gen4 error: Column 'col1' is ambiguous
 
 # non-ambiguous symbol reference
 "select user.col1, user_extra.col1 from user join user_extra having user_extra.col1 = 2"
@@ -56,6 +72,40 @@
     ]
   }
 }
+{
+  "QueryType": "SELECT",
+  "Original": "select user.col1, user_extra.col1 from user join user_extra having user_extra.col1 = 2",
+  "Instructions": {
+    "OperatorType": "Join",
+    "Variant": "Join",
+    "JoinColumnIndexes": "-1,1",
+    "TableName": "`user`_user_extra",
+    "Inputs": [
+      {
+        "OperatorType": "Route",
+        "Variant": "SelectScatter",
+        "Keyspace": {
+          "Name": "user",
+          "Sharded": true
+        },
+        "FieldQuery": "select `user`.col1 from `user` where 1 != 1",
+        "Query": "select `user`.col1 from `user`",
+        "Table": "`user`"
+      },
+      {
+        "OperatorType": "Route",
+        "Variant": "SelectScatter",
+        "Keyspace": {
+          "Name": "user",
+          "Sharded": true
+        },
+        "FieldQuery": "select user_extra.col1 from user_extra where 1 != 1",
+        "Query": "select user_extra.col1 from user_extra where user_extra.col1 = 2",
+        "Table": "user_extra"
+      }
+    ]
+  }
+}
 
 # HAVING multi-route
 "select user.col1 as a, user.col2, user_extra.col3 from user join user_extra having 1 = 1 and a = 1 and a = user.col2 and user_extra.col3 = 1"
@@ -93,6 +143,40 @@
     ]
   }
 }
+{
+  "QueryType": "SELECT",
+  "Original": "select user.col1 as a, user.col2, user_extra.col3 from user join user_extra having 1 = 1 and a = 1 and a = user.col2 and user_extra.col3 = 1",
+  "Instructions": {
+    "OperatorType": "Join",
+    "Variant": "Join",
+    "JoinColumnIndexes": "-1,-2,1",
+    "TableName": "`user`_user_extra",
+    "Inputs": [
+      {
+        "OperatorType": "Route",
+        "Variant": "SelectScatter",
+        "Keyspace": {
+          "Name": "user",
+          "Sharded": true
+        },
+        "FieldQuery": "select `user`.col1 as a, `user`.col2 from `user` where 1 != 1",
+        "Query": "select `user`.col1 as a, `user`.col2 from `user` where `user`.col1 = 1 and `user`.col1 = `user`.col2 and 1 = 1",
+        "Table": "`user`"
+      },
+      {
+        "OperatorType": "Route",
+        "Variant": "SelectScatter",
+        "Keyspace": {
+          "Name": "user",
+          "Sharded": true
+        },
+        "FieldQuery": "select user_extra.col3 from user_extra where 1 != 1",
+        "Query": "select user_extra.col3 from user_extra where user_extra.col3 = 1 and 1 = 1",
+        "Table": "user_extra"
+      }
+    ]
+  }
+}
 
 # HAVING uses subquery
 "select id from user having id in (select col from user)"
@@ -132,6 +216,42 @@
     ]
   }
 }
+{
+  "QueryType": "SELECT",
+  "Original": "select id from user having id in (select col from user)",
+  "Instructions": {
+    "OperatorType": "Subquery",
+    "Variant": "PulloutIn",
+    "Inputs": [
+      {
+        "OperatorType": "Route",
+        "Variant": "SelectScatter",
+        "Keyspace": {
+          "Name": "user",
+          "Sharded": true
+        },
+        "FieldQuery": "select col from `user` where 1 != 1",
+        "Query": "select col from `user`",
+        "Table": "`user`"
+      },
+      {
+        "OperatorType": "Route",
+        "Variant": "SelectIN",
+        "Keyspace": {
+          "Name": "user",
+          "Sharded": true
+        },
+        "FieldQuery": "select id from `user` where 1 != 1",
+        "Query": "select id from `user` where id in ::__vals",
+        "Table": "`user`",
+        "Values": [
+          "::__sq1"
+        ],
+        "Vindex": "user_index"
+      }
+    ]
+  }
+}
 
 # ORDER BY, reference col from local table.
 "select col from user where id = 5 order by aa"
@@ -1094,6 +1214,21 @@ Gen4 plan same as above
 # Order by, verify outer symtab is searched according to its own context.
 "select u.id from user u having u.id in (select col2 from user where user.id = u.id order by u.col)"
 "symbol u.col not found in subquery"
+{
+  "QueryType": "SELECT",
+  "Original": "select u.id from user u having u.id in (select col2 from user where user.id = u.id order by u.col)",
+  "Instructions": {
+    "OperatorType": "Route",
+    "Variant": "SelectScatter",
+    "Keyspace": {
+      "Name": "user",
+      "Sharded": true
+    },
+    "FieldQuery": "select u.id from `user` as u where 1 != 1",
+    "Query": "select u.id from `user` as u where u.id in (select col2 from `user` where `user`.id = u.id order by u.col asc)",
+    "Table": "`user`"
+  }
+}
 
 # Order by, qualified '*' expression, name mismatched.
 "select user.* from user where id = 5 order by e.col"
diff --git a/go/vt/vtgate/planbuilder/testdata/unsupported_cases.txt b/go/vt/vtgate/planbuilder/testdata/unsupported_cases.txt
index 48a70e71e2..1cdf561574 100644
--- a/go/vt/vtgate/planbuilder/testdata/unsupported_cases.txt
+++ b/go/vt/vtgate/planbuilder/testdata/unsupported_cases.txt
@@ -87,6 +87,7 @@ Gen4 plan same as above
 # Filtering on scatter aggregates
 "select count(*) a from user having a >10"
 "unsupported: filtering on results of aggregates"
+Gen4 plan same as above
 
 # group by must reference select list
 "select a from user group by b"
@@ -492,3 +493,54 @@ Gen4 plan same as above
 "select count(*) from user join user_extra"
 "unsupported: cross-shard query with aggregates"
 Gen4 plan same as above
+
+# TODO this should be planned without using OA and MS
+"select u.id from user u join user_extra ue on ue.id = u.id group by u.id having count(u.name) = 3"
+"unsupported: cross-shard query with aggregates"
+Gen4 error: unsupported: filtering on results of aggregates
+
+"select (select 1 from user u having count(ue.col) > 10) from user_extra ue"
+"symbol ue.col not found in subquery"
+Gen4 error: unsupported: filtering on results of aggregates
+
+# aggregation filtering by having on a route with no group by
+"select 1 from user having count(id) = 10"
+{
+  "QueryType": "SELECT",
+  "Original": "select 1 from user having count(id) = 10",
+  "Instructions": {
+    "OperatorType": "Route",
+    "Variant": "SelectScatter",
+    "Keyspace": {
+      "Name": "user",
+      "Sharded": true
+    },
+    "FieldQuery": "select 1 from `user` where 1 != 1",
+    "Query": "select 1 from `user` having count(id) = 10",
+    "Table": "`user`"
+  }
+}
+Gen4 error: unsupported: filtering on results of aggregates
+
+# aggregation filtering by having on a route with no group by with non-unique vindex filter
+"select 1 from user having count(id) = 10 and name = 'a'"
+{
+  "QueryType": "SELECT",
+  "Original": "select 1 from user having count(id) = 10 and name = 'a'",
+  "Instructions": {
+    "OperatorType": "Route",
+    "Variant": "SelectEqual",
+    "Keyspace": {
+      "Name": "user",
+      "Sharded": true
+    },
+    "FieldQuery": "select 1 from `user` where 1 != 1",
+    "Query": "select 1 from `user` having count(id) = 10 and `name` = 'a'",
+    "Table": "`user`",
+    "Values": [
+      "a"
+    ],
+    "Vindex": "name_user_map"
+  }
+}
+Gen4 error: unsupported: filtering on results of aggregates
diff --git a/go/vt/vtgate/planbuilder/testdata/wireup_cases.txt b/go/vt/vtgate/planbuilder/testdata/wireup_cases.txt
index 728e9e8611..cc2ef8b3ed 100644
--- a/go/vt/vtgate/planbuilder/testdata/wireup_cases.txt
+++ b/go/vt/vtgate/planbuilder/testdata/wireup_cases.txt
@@ -37,6 +37,48 @@
     ]
   }
 }
+{
+  "QueryType": "SELECT",
+  "Original": "select e.col, u.id uid, e.id eid from user u join user_extra e having uid = eid",
+  "Instructions": {
+    "OperatorType": "Join",
+    "Variant": "Join",
+    "JoinColumnIndexes": "-2,1,-1",
+    "JoinVars": {
+      "e_id": 0
+    },
+    "TableName": "user_extra_`user`",
+    "Inputs": [
+      {
+        "OperatorType": "Route",
+        "Variant": "SelectScatter",
+        "Keyspace": {
+          "Name": "user",
+          "Sharded": true
+        },
+        "FieldQuery": "select e.id, e.col from user_extra as e where 1 != 1",
+        "Query": "select e.id, e.col from user_extra as e",
+        "Table": "user_extra"
+      },
+      {
+        "OperatorType": "Route",
+        "Variant": "SelectEqualUnique",
+        "Keyspace": {
+          "Name": "user",
+          "Sharded": true
+        },
+        "FieldQuery": "select u.id as uid from `user` as u where 1 != 1",
+        "Query": "select u.id as uid from `user` as u where u.id = :e_id",
+        "Table": "`user`",
+        "Values": [
+          ":e_id"
+        ],
+        "Vindex": "user_index"
+      }
+    ]
+  }
+}
+
 
 # bind var already in use
 "select e.col, u.id uid, e.id eid from user u join user_extra e having uid = eid and e.col = :uid"
@@ -77,6 +119,47 @@
     ]
   }
 }
+{
+  "QueryType": "SELECT",
+  "Original": "select e.col, u.id uid, e.id eid from user u join user_extra e having uid = eid and e.col = :uid",
+  "Instructions": {
+    "OperatorType": "Join",
+    "Variant": "Join",
+    "JoinColumnIndexes": "-2,1,-1",
+    "JoinVars": {
+      "e_id": 0
+    },
+    "TableName": "user_extra_`user`",
+    "Inputs": [
+      {
+        "OperatorType": "Route",
+        "Variant": "SelectScatter",
+        "Keyspace": {
+          "Name": "user",
+          "Sharded": true
+        },
+        "FieldQuery": "select e.id, e.col from user_extra as e where 1 != 1",
+        "Query": "select e.id, e.col from user_extra as e where e.col = :uid",
+        "Table": "user_extra"
+      },
+      {
+        "OperatorType": "Route",
+        "Variant": "SelectEqualUnique",
+        "Keyspace": {
+          "Name": "user",
+          "Sharded": true
+        },
+        "FieldQuery": "select u.id as uid from `user` as u where 1 != 1",
+        "Query": "select u.id as uid from `user` as u where u.id = :e_id",
+        "Table": "`user`",
+        "Values": [
+          ":e_id"
+        ],
+        "Vindex": "user_index"
+      }
+    ]
+  }
+}
 
 # wire-up join with join, going left
 "select u1.id from user u1 join user u2 join user u3 where u3.col = u1.col"
diff --git a/go/vt/vtgate/semantics/analyzer_test.go b/go/vt/vtgate/semantics/analyzer_test.go
index 98f45ce1ac..0164cd7d01 100644
--- a/go/vt/vtgate/semantics/analyzer_test.go
+++ b/go/vt/vtgate/semantics/analyzer_test.go
@@ -191,7 +191,7 @@ func TestOrderByBindingMultiTable(t *testing.T) {
 	})
 }
 
-func TestGroupByBindingSingleTable(t *testing.T) {
+func TestGroupByBinding(t *testing.T) {
 	tcases := []struct {
 		sql  string
 		deps TableSet
@@ -237,6 +237,55 @@ func TestGroupByBindingSingleTable(t *testing.T) {
 	}
 }
 
+func TestHavingBinding(t *testing.T) {
+	tcases := []struct {
+		sql  string
+		deps TableSet
+	}{{
+		"select col from tabl having col = 1",
+		T1,
+	}, {
+		"select col from tabl having tabl.col = 1",
+		T1,
+	}, {
+		"select col from tabl having d.tabl.col = 1",
+		T1,
+	}, {
+		"select tabl.col as x from tabl having x = 1",
+		T1,
+	}, {
+		"select tabl.col as x from tabl having col",
+		T1,
+	}, {
+		"select col from tabl having 1 = 1",
+		T0,
+	}, {
+		"select col as c from tabl having c = 1",
+		T1,
+	}, {
+		"select 1 as c from tabl having c = 1",
+		T0,
+	}, {
+		"select t1.id from t1, t2 having id = 1",
+		T1,
+	}, {
+		"select t.id from t, t1 having id = 1",
+		T1,
+	}, {
+		"select t.id, count(*) as a from t, t1 group by t.id having a = 1",
+		T1 | T2,
+	}}
+	for _, tc := range tcases {
+		t.Run(tc.sql, func(t *testing.T) {
+			stmt, semTable := parseAndAnalyze(t, tc.sql, "d")
+			sel, _ := stmt.(*sqlparser.Select)
+			hvng := sel.Having.Expr
+			d := semTable.BaseTableDependencies(hvng)
+			require.Equal(t, tc.deps, d, tc.sql)
+		})
+	}
+}
+
 func TestBindingSingleAliasedTable(t *testing.T) {
 	t.Run("positive tests", func(t *testing.T) {
 		queries := []string{