Repository: vitessio/vitess
Pull request #11772 (2022-11-18 07:23:54)

test: deflake TestQueryTimeoutWithTables

<!--
  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. -->

This PR deflakes the test added in `Query Timeout` feature.

## Related Issue(s)

<!-- List related issues and pull requests. If this PR fixes an issue, please add it using Fixes #????  -->

## Checklist

-   [X] Tests were added or are not required
-   [X] Documentation was added or is not required



vitess-bot[bot]: ### Review Checklist
            
Hello reviewers! :wave: Please follow this checklist when reviewing this Pull Request.

#### General
- [ ] Ensure that the Pull Request has a descriptive title.
- [ ] If this is a change that users need to know about, please apply the `release notes (needs details)` label so that merging is blocked unless the summary release notes document is included.

#### If a new flag is being introduced:
- [ ] Is it really necessary to add this flag?
- [ ] Flag names should be clear and intuitive (as far as possible)
- [ ] Help text should be descriptive.
- [ ] Flag names should use dashes (`-`) as word separators rather than underscores (`_`).

#### If a workflow is added or modified:
- [ ] Each item in `Jobs` should be named in order to mark it as `required`.
- [ ] If the workflow should be required, the maintainer team should be notified.

#### Bug fixes
- [ ] There should be at least one unit or end-to-end test.
- [ ] The Pull Request description should include a link to an issue that describes the bug.

#### Non-trivial changes
- [ ] There should be some code comments as to why things are implemented the way they are.

#### New/Existing features
- [ ] Should be documented, either by modifying the existing documentation or creating new documentation.
- [ ] New features should have a link to a feature request issue or an RFC that documents the use cases, corner cases and test cases.

#### Backward compatibility            
- [ ] Protobuf changes should be wire-compatible.
- [ ] Changes to `_vt` tables and RPCs need to be backward compatible.
- [ ] `vtctl` command output order should be stable and `awk`-able.
- [ ] RPC changes should be compatible with vitess-operator
- [ ] If a flag is removed, then it should also be removed from VTop, if used there.


Patch:
diff --git a/go/vt/sqlparser/ast_rewriting.go b/go/vt/sqlparser/ast_rewriting.go
index 3fa4d0d6d9..101477974f 100644
--- a/go/vt/sqlparser/ast_rewriting.go
+++ b/go/vt/sqlparser/ast_rewriting.go
@@ -54,7 +54,7 @@ type ReservedVars struct {
 }
 
 // ReserveAll tries to reserve all the given variable names. If they're all available,
-// they are reserved and the function returns true. Otherwise the function returns false.
+// they are reserved and the function returns true. Otherwise, the function returns false.
 func (r *ReservedVars) ReserveAll(names ...string) bool {
 	for _, name := range names {
 		if _, ok := r.reserved[name]; ok {
@@ -615,188 +615,3 @@ func SystemSchema(schema string) bool {
 		strings.EqualFold(schema, "sys") ||
 		strings.EqualFold(schema, "mysql")
 }
-
-// RewriteToCNF walks the input AST and rewrites any boolean logic into CNF
-// Note: In order to re-plan, we need to empty the accumulated metadata in the AST,
-// so ColName.Metadata will be nil:ed out as part of this rewrite
-func RewriteToCNF(ast SQLNode) SQLNode {
-	for {
-		finishedRewrite := true
-		ast = Rewrite(ast, func(cursor *Cursor) bool {
-			if e, isExpr := cursor.node.(Expr); isExpr {
-				rewritten, didRewrite := rewriteToCNFExpr(e)
-				if didRewrite {
-					finishedRewrite = false
-					cursor.Replace(rewritten)
-				}
-			}
-			if col, isCol := cursor.node.(*ColName); isCol {
-				col.Metadata = nil
-			}
-			return true
-		}, nil)
-
-		if finishedRewrite {
-			return ast
-		}
-	}
-}
-
-func distinctOr(in *OrExpr) (Expr, bool) {
-	todo := []*OrExpr{in}
-	var leaves []Expr
-	for len(todo) > 0 {
-		curr := todo[0]
-		todo = todo[1:]
-		addAnd := func(in Expr) {
-			and, ok := in.(*OrExpr)
-			if ok {
-				todo = append(todo, and)
-			} else {
-				leaves = append(leaves, in)
-			}
-		}
-		addAnd(curr.Left)
-		addAnd(curr.Right)
-	}
-	original := len(leaves)
-	var predicates []Expr
-
-outer1:
-	for len(leaves) > 0 {
-		curr := leaves[0]
-		leaves = leaves[1:]
-		for _, alreadyIn := range predicates {
-			if EqualsExpr(alreadyIn, curr) {
-				continue outer1
-			}
-		}
-		predicates = append(predicates, curr)
-	}
-	if original == len(predicates) {
-		return in, false
-	}
-	var result Expr
-	for i, curr := range predicates {
-		if i == 0 {
-			result = curr
-			continue
-		}
-		result = &OrExpr{Left: result, Right: curr}
-	}
-	return result, true
-}
-func distinctAnd(in *AndExpr) (Expr, bool) {
-	todo := []*AndExpr{in}
-	var leaves []Expr
-	for len(todo) > 0 {
-		curr := todo[0]
-		todo = todo[1:]
-		addAnd := func(in Expr) {
-			and, ok := in.(*AndExpr)
-			if ok {
-				todo = append(todo, and)
-			} else {
-				leaves = append(leaves, in)
-			}
-		}
-		addAnd(curr.Left)
-		addAnd(curr.Right)
-	}
-	original := len(leaves)
-	var predicates []Expr
-
-outer1:
-	for len(leaves) > 0 {
-		curr := leaves[0]
-		leaves = leaves[1:]
-		for _, alreadyIn := range predicates {
-			if EqualsExpr(alreadyIn, curr) {
-				continue outer1
-			}
-		}
-		predicates = append(predicates, curr)
-	}
-	if original == len(predicates) {
-		return in, false
-	}
-	var result Expr
-	for i, curr := range predicates {
-		if i == 0 {
-			result = curr
-			continue
-		}
-		result = &AndExpr{Left: result, Right: curr}
-	}
-	return result, true
-}
-
-func rewriteToCNFExpr(expr Expr) (Expr, bool) {
-	switch expr := expr.(type) {
-	case *NotExpr:
-		switch child := expr.Expr.(type) {
-		case *NotExpr:
-			// NOT NOT A => A
-			return child.Expr, true
-		case *OrExpr:
-			// DeMorgan Rewriter
-			// NOT (A OR B) => NOT A AND NOT B
-			return &AndExpr{Right: &NotExpr{Expr: child.Right}, Left: &NotExpr{Expr: child.Left}}, true
-		case *AndExpr:
-			// DeMorgan Rewriter
-			// NOT (A AND B) => NOT A OR NOT B
-			return &OrExpr{Right: &NotExpr{Expr: child.Right}, Left: &NotExpr{Expr: child.Left}}, true
-		}
-	case *OrExpr:
-		or := expr
-		if and, ok := or.Left.(*AndExpr); ok {
-			// Simplification
-			// (A AND B) OR A => A
-			if EqualsExpr(or.Right, and.Left) || EqualsExpr(or.Right, and.Right) {
-				return or.Right, true
-			}
-			// Distribution Law
-			// (A AND B) OR C => (A OR C) AND (B OR C)
-			return &AndExpr{Left: &OrExpr{Left: and.Left, Right: or.Right}, Right: &OrExpr{Left: and.Right, Right: or.Right}}, true
-		}
-		if and, ok := or.Right.(*AndExpr); ok {
-			// Simplification
-			// A OR (A AND B) => A
-			if EqualsExpr(or.Left, and.Left) || EqualsExpr(or.Left, and.Right) {
-				return or.Left, true
-			}
-			// Distribution Law
-			// C OR (A AND B) => (C OR A) AND (C OR B)
-			return &AndExpr{Left: &OrExpr{Left: or.Left, Right: and.Left}, Right: &OrExpr{Left: or.Left, Right: and.Right}}, true
-		}
-		// Try to make distinct
-		return distinctOr(expr)
-
-	case *XorExpr:
-		// DeMorgan Rewriter
-		// (A XOR B) => (A OR B) AND NOT (A AND B)
-		return &AndExpr{Left: &OrExpr{Left: expr.Left, Right: expr.Right}, Right: &NotExpr{Expr: &AndExpr{Left: expr.Left, Right: expr.Right}}}, true
-	case *AndExpr:
-		res, rewritten := distinctAnd(expr)
-		if rewritten {
-			return res, rewritten
-		}
-		and := expr
-		if or, ok := and.Left.(*OrExpr); ok {
-			// Simplification
-			// (A OR B) AND A => A
-			if EqualsExpr(or.Left, and.Right) || EqualsExpr(or.Right, and.Right) {
-				return and.Right, true
-			}
-		}
-		if or, ok := and.Right.(*OrExpr); ok {
-			// Simplification
-			// A OR (A AND B) => A
-			if EqualsExpr(or.Left, and.Left) || EqualsExpr(or.Right, and.Left) {
-				return or.Left, true
-			}
-		}
-
-	}
-	return expr, false
-}
diff --git a/go/vt/sqlparser/predicate_rewriting.go b/go/vt/sqlparser/predicate_rewriting.go
new file mode 100644
index 0000000000..2c21222d12
--- /dev/null
+++ b/go/vt/sqlparser/predicate_rewriting.go
@@ -0,0 +1,317 @@
+/*
+Copyright 2022 The Vitess Authors.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package sqlparser
+
+const (
+	Changed  RewriteState = true
+	NoChange RewriteState = false
+)
+
+type RewriteState bool
+
+// RewritePredicate walks the input AST and rewrites any boolean logic into a simpler form
+// This simpler form is CNF plus logic for extracting predicates from OR, plus logic for turning ORs into IN
+// Note: In order to re-plan, we need to empty the accumulated metadata in the AST,
+// so ColName.Metadata will be nil:ed out as part of this rewrite
+func RewritePredicate(ast SQLNode) SQLNode {
+	for {
+		finishedRewrite := true
+		ast = Rewrite(ast, nil, func(cursor *Cursor) bool {
+			if e, isExpr := cursor.node.(Expr); isExpr {
+				rewritten, state := simplifyExpression(e)
+				if state == Changed {
+					finishedRewrite = false
+					cursor.Replace(rewritten)
+				}
+			}
+			if col, isCol := cursor.node.(*ColName); isCol {
+				col.Metadata = nil
+			}
+			return true
+		})
+
+		if finishedRewrite {
+			return ast
+		}
+	}
+}
+
+func simplifyExpression(expr Expr) (Expr, RewriteState) {
+	switch expr := expr.(type) {
+	case *NotExpr:
+		return simplifyNot(expr)
+	case *OrExpr:
+		return simplifyOr(expr)
+	case *XorExpr:
+		return simplifyXor(expr)
+	case *AndExpr:
+		return simplifyAnd(expr)
+	}
+	return expr, NoChange
+}
+
+func simplifyNot(expr *NotExpr) (Expr, RewriteState) {
+	switch child := expr.Expr.(type) {
+	case *NotExpr:
+		// NOT NOT A => A
+		return child.Expr, Changed
+	case *OrExpr:
+		// DeMorgan Rewriter
+		// NOT (A OR B) => NOT A AND NOT B
+		return &AndExpr{Right: &NotExpr{Expr: child.Right}, Left: &NotExpr{Expr: child.Left}}, Changed
+	case *AndExpr:
+		// DeMorgan Rewriter
+		// NOT (A AND B) => NOT A OR NOT B
+		return &OrExpr{Right: &NotExpr{Expr: child.Right}, Left: &NotExpr{Expr: child.Left}}, Changed
+	}
+	return expr, NoChange
+}
+
+func simplifyOr(expr *OrExpr) (Expr, RewriteState) {
+	or := expr
+
+	// first we search for ANDs and see how they can be simplified
+	land, lok := or.Left.(*AndExpr)
+	rand, rok := or.Right.(*AndExpr)
+	switch {
+	case lok && rok:
+		var a, b, c Expr
+		switch {
+		// (A and B) or (A and C) => A AND (B OR C)
+		case EqualsExpr(land.Left, rand.Left):
+			a, b, c = land.Left, land.Right, rand.Right
+		// (A and B) or (C and A) => A AND (B OR C)
+		case EqualsExpr(land.Left, rand.Right):
+			a, b, c = land.Left, land.Right, rand.Left
+		// (B and A) or (A and C) => A AND (B OR C)
+		case EqualsExpr(land.Right, rand.Left):
+			a, b, c = land.Right, land.Left, rand.Right
+		// (B and A) or (C and A) => A AND (B OR C)
+		case EqualsExpr(land.Right, rand.Right):
+			a, b, c = land.Right, land.Left, rand.Left
+		default:
+			return expr, NoChange
+		}
+		return &AndExpr{Left: a, Right: &OrExpr{Left: b, Right: c}}, Changed
+	case lok:
+		// Simplification
+		// (A AND B) OR A => A
+		if EqualsExpr(or.Right, land.Left) || EqualsExpr(or.Right, land.Right) {
+			return or.Right, Changed
+		}
+		// Distribution Law
+		// (A AND B) OR C => (A OR C) AND (B OR C)
+		return &AndExpr{Left: &OrExpr{Left: land.Left, Right: or.Right}, Right: &OrExpr{Left: land.Right, Right: or.Right}}, Changed
+	case rok:
+		// Simplification
+		// A OR (A AND B) => A
+		if EqualsExpr(or.Left, rand.Left) || EqualsExpr(or.Left, rand.Right) {
+			return or.Left, Changed
+		}
+		// Distribution Law
+		// C OR (A AND B) => (C OR A) AND (C OR B)
+		return &AndExpr{Left: &OrExpr{Left: or.Left, Right: rand.Left}, Right: &OrExpr{Left: or.Left, Right: rand.Right}}, Changed
+	}
+
+	// next, we want to try to turn multiple ORs into an IN when possible
+	lftCmp, lok := or.Left.(*ComparisonExpr)
+	rgtCmp, rok := or.Right.(*ComparisonExpr)
+	if lok && rok {
+		newExpr, rewritten := tryTurningOrIntoIn(lftCmp, rgtCmp)
+		if rewritten {
+			return newExpr, Changed
+		}
+	}
+
+	// Try to make distinct
+	return distinctOr(expr)
+}
+
+func tryTurningOrIntoIn(l, r *ComparisonExpr) (Expr, RewriteState) {
+	// looks for A = X OR A = Y and turns them into A IN (X, Y)
+	col, ok := l.Left.(*ColName)
+	if !ok || !EqualsExpr(col, r.Left) {
+		return nil, NoChange
+	}
+
+	var tuple ValTuple
+
+	switch l.Operator {
+	case EqualOp:
+		tuple = ValTuple{l.Right}
+	case InOp:
+		lft, ok := l.Right.(ValTuple)
+		if !ok {
+			return nil, NoChange
+		}
+		tuple = lft
+	default:
+		return nil, NoChange
+	}
+
+	switch r.Operator {
+	case EqualOp:
+		tuple = append(tuple, r.Right)
+	case InOp:
+		lft, ok := r.Right.(ValTuple)
+		if !ok {
+			return nil, NoChange
+		}
+		tuple = append(tuple, lft...)
+	default:
+		return nil, NoChange
+	}
+
+	return &ComparisonExpr{
+		Operator: InOp,
+		Left:     col,
+		Right:    uniquefy(tuple),
+	}, Changed
+}
+
+func uniquefy(tuple ValTuple) (output ValTuple) {
+outer:
+	for _, expr := range tuple {
+		for _, seen := range output {
+			if EqualsExpr(expr, seen) {
+				continue outer
+			}
+		}
+		output = append(output, expr)
+	}
+	return
+}
+
+func simplifyXor(expr *XorExpr) (Expr, RewriteState) {
+	// DeMorgan Rewriter
+	// (A XOR B) => (A OR B) AND NOT (A AND B)
+	return &AndExpr{Left: &OrExpr{Left: expr.Left, Right: expr.Right}, Right: &NotExpr{Expr: &AndExpr{Left: expr.Left, Right: expr.Right}}}, Changed
+}
+
+func simplifyAnd(expr *AndExpr) (Expr, RewriteState) {
+	res, rewritten := distinctAnd(expr)
+	if rewritten {
+		return res, rewritten
+	}
+	and := expr
+	if or, ok := and.Left.(*OrExpr); ok {
+		// Simplification
+		// (A OR B) AND A => A
+		if EqualsExpr(or.Left, and.Right) || EqualsExpr(or.Right, and.Right) {
+			return and.Right, Changed
+		}
+	}
+	if or, ok := and.Right.(*OrExpr); ok {
+		// Simplification
+		// A OR (A AND B) => A
+		if EqualsExpr(or.Left, and.Left) || EqualsExpr(or.Right, and.Left) {
+			return or.Left, Changed
+		}
+	}
+
+	return expr, NoChange
+}
+
+func distinctOr(in *OrExpr) (Expr, RewriteState) {
+	todo := []*OrExpr{in}
+	var leaves []Expr
+	for len(todo) > 0 {
+		curr := todo[0]
+		todo = todo[1:]
+		addAnd := func(in Expr) {
+			and, ok := in.(*OrExpr)
+			if ok {
+				todo = append(todo, and)
+			} else {
+				leaves = append(leaves, in)
+			}
+		}
+		addAnd(curr.Left)
+		addAnd(curr.Right)
+	}
+	original := len(leaves)
+	var predicates []Expr
+
+outer1:
+	for len(leaves) > 0 {
+		curr := leaves[0]
+		leaves = leaves[1:]
+		for _, alreadyIn := range predicates {
+			if EqualsExpr(alreadyIn, curr) {
+				continue outer1
+			}
+		}
+		predicates = append(predicates, curr)
+	}
+	if original == len(predicates) {
+		return in, NoChange
+	}
+	var result Expr
+	for i, curr := range predicates {
+		if i == 0 {
+			result = curr
+			continue
+		}
+		result = &OrExpr{Left: result, Right: curr}
+	}
+	return result, Changed
+}
+
+func distinctAnd(in *AndExpr) (Expr, RewriteState) {
+	todo := []*AndExpr{in}
+	var leaves []Expr
+	for len(todo) > 0 {
+		curr := todo[0]
+		todo = todo[1:]
+		addAnd := func(in Expr) {
+			and, ok := in.(*AndExpr)
+			if ok {
+				todo = append(todo, and)
+			} else {
+				leaves = append(leaves, in)
+			}
+		}
+		addAnd(curr.Left)
+		addAnd(curr.Right)
+	}
+	original := len(leaves)
+	var predicates []Expr
+
+outer1:
+	for len(leaves) > 0 {
+		curr := leaves[0]
+		leaves = leaves[1:]
+		for _, alreadyIn := range predicates {
+			if EqualsExpr(alreadyIn, curr) {
+				continue outer1
+			}
+		}
+		predicates = append(predicates, curr)
+	}
+	if original == len(predicates) {
+		return in, NoChange
+	}
+	var result Expr
+	for i, curr := range predicates {
+		if i == 0 {
+			result = curr
+			continue
+		}
+		result = &AndExpr{Left: result, Right: curr}
+	}
+	return result, Changed
+}
diff --git a/go/vt/vtgate/planbuilder/ddl.go b/go/vt/vtgate/planbuilder/ddl.go
index acadccdc82..700b2f2c1d 100644
--- a/go/vt/vtgate/planbuilder/ddl.go
+++ b/go/vt/vtgate/planbuilder/ddl.go
@@ -45,7 +45,7 @@ func (fk *fkContraint) FkWalk(node sqlparser.SQLNode) (kontinue bool, err error)
 }
 
 // buildGeneralDDLPlan builds a general DDL plan, which can be either normal DDL or online DDL.
-// The two behave compeltely differently, and have two very different primitives.
+// The two behave completely differently, and have two very different primitives.
 // We want to be able to dynamically choose between normal/online plans according to Session settings.
 // However, due to caching of plans, we're unable to make that choice right now. In this function we don't have
 // a session context. It's only when we Execute() the primitive that we have that context.
diff --git a/go/vt/vtgate/planbuilder/gen4_planner.go b/go/vt/vtgate/planbuilder/gen4_planner.go
index 6d9a7ae28f..be9d29f60f 100644
--- a/go/vt/vtgate/planbuilder/gen4_planner.go
+++ b/go/vt/vtgate/planbuilder/gen4_planner.go
@@ -30,8 +30,6 @@ import (
 	"vitess.io/vitess/go/vt/vtgate/vindexes"
 )
 
-var _ stmtPlanner = gen4Planner("apa", 0)
-
 func gen4Planner(query string, plannerVersion querypb.ExecuteOptions_PlannerVersion) stmtPlanner {
 	return func(stmt sqlparser.Statement, reservedVars *sqlparser.ReservedVars, vschema plancontext.VSchema) (*planResult, error) {
 		switch stmt := stmt.(type) {
@@ -99,9 +97,9 @@ func gen4SelectStmtPlanner(
 		return nil, err
 	}
 
-	if shouldRetryWithCNFRewriting(plan) {
+	if shouldRetryAfterPredicateRewriting(plan) {
 		// by transforming the predicates to CNF, the planner will sometimes find better plans
-		primitive, st := gen4CNFRewrite(stmt, getPlan)
+		primitive, st := gen4PredicateRewrite(stmt, getPlan)
 		if primitive != nil {
 			return newPlanResult(primitive, tablesFromSemantics(st)...), nil
 		}
@@ -154,14 +152,14 @@ func planSelectGen4(reservedVars *sqlparser.ReservedVars, vschema plancontext.VS
 	return nil, plan, nil
 }
 
-func gen4CNFRewrite(stmt sqlparser.Statement, getPlan func(selStatement sqlparser.SelectStatement) (logicalPlan, *semantics.SemTable, error)) (engine.Primitive, *semantics.SemTable) {
-	rewritten, isSel := sqlparser.RewriteToCNF(stmt).(sqlparser.SelectStatement)
+func gen4PredicateRewrite(stmt sqlparser.Statement, getPlan func(selStatement sqlparser.SelectStatement) (logicalPlan, *semantics.SemTable, error)) (engine.Primitive, *semantics.SemTable) {
+	rewritten, isSel := sqlparser.RewritePredicate(stmt).(sqlparser.SelectStatement)
 	if !isSel {
 		// Fail-safe code, should never happen
 		return nil, nil
 	}
 	plan2, st, err := getPlan(rewritten)
-	if err == nil && !shouldRetryWithCNFRewriting(plan2) {
+	if err == nil && !shouldRetryAfterPredicateRewriting(plan2) {
 		// we only use this new plan if it's better than the old one we got
 		return plan2.Primitive(), st
 	}
diff --git a/go/vt/vtgate/planbuilder/operators/route.go b/go/vt/vtgate/planbuilder/operators/route.go
index 2015b71f2c..b2dce9b398 100644
--- a/go/vt/vtgate/planbuilder/operators/route.go
+++ b/go/vt/vtgate/planbuilder/operators/route.go
@@ -791,10 +791,15 @@ func createRoute(ctx *plancontext.PlanningContext, table *QueryTable, solves sem
 
 	if plan.RouteOpCode == engine.Scatter && len(table.Predicates) > 0 {
 		// If we have a scatter query, it's worth spending a little extra time seeing if we can't improve it
-		for _, pred := range table.Predicates {
-			rewritten := tryRewriteOrToIn(pred)
-			if rewritten != nil {
-				err = plan.UpdateRoutingLogic(ctx, rewritten)
+		oldPredicates := table.Predicates
+		table.Predicates = nil
+		plan.SeenPredicates = nil
+		for _, pred := range oldPredicates {
+			rewritten := sqlparser.RewritePredicate(pred)
+			predicates := sqlparser.SplitAndExpression(nil, rewritten.(sqlparser.Expr))
+			for _, predicate := range predicates {
+				table.Predicates = append(table.Predicates, predicate)
+				err = plan.UpdateRoutingLogic(ctx, predicate)
 				if err != nil {
 					return nil, err
 				}
diff --git a/go/vt/vtgate/planbuilder/operators/route_planning.go b/go/vt/vtgate/planbuilder/operators/route_planning.go
index e121c8b8fe..3100f1ef40 100644
--- a/go/vt/vtgate/planbuilder/operators/route_planning.go
+++ b/go/vt/vtgate/planbuilder/operators/route_planning.go
@@ -272,71 +272,6 @@ func seedOperatorList(ctx *plancontext.PlanningContext, qg *QueryGraph) ([]ops.O
 	return plans, nil
 }
 
-func tryRewriteOrToIn(expr sqlparser.Expr) sqlparser.Expr {
-	rewrote := false
-	newPred := sqlparser.Rewrite(sqlparser.CloneExpr(expr), func(cursor *sqlparser.Cursor) bool {
-		_, ok := cursor.Node().(*sqlparser.OrExpr)
-		return ok
-	}, func(cursor *sqlparser.Cursor) bool {
-		// we are looking for the pattern WHERE c = 1 or c = 2
-		switch or := cursor.Node().(type) {
-		case *sqlparser.OrExpr:
-			lftCmp, ok := or.Left.(*sqlparser.ComparisonExpr)
-			if !ok {
-				return true
-			}
-			rgtCmp, ok := or.Right.(*sqlparser.ComparisonExpr)
-			if !ok {
-				return true
-			}
-
-			col, ok := lftCmp.Left.(*sqlparser.ColName)
-			if !ok || !sqlparser.EqualsExpr(lftCmp.Left, rgtCmp.Left) {
-				return true
-			}
-
-			var tuple sqlparser.ValTuple
-			switch lftCmp.Operator {
-			case sqlparser.EqualOp:
-				tuple = sqlparser.ValTuple{lftCmp.Right}
-			case sqlparser.InOp:
-				lft, ok := lftCmp.Right.(sqlparser.ValTuple)
-				if !ok {
-					return true
-				}
-				tuple = lft
-			default:
-				return true
-			}
-
-			switch rgtCmp.Operator {
-			case sqlparser.EqualOp:
-				tuple = append(tuple, rgtCmp.Right)
-			case sqlparser.InOp:
-				lft, ok := rgtCmp.Right.(sqlparser.ValTuple)
-				if !ok {
-					return true
-				}
-				tuple = append(tuple, lft...)
-			default:
-				return true
-			}
-
-			rewrote = true
-			cursor.Replace(&sqlparser.ComparisonExpr{
-				Operator: sqlparser.InOp,
-				Left:     col,
-				Right:    tuple,
-			})
-		}
-		return true
-	})
-	if rewrote {
-		return newPred.(sqlparser.Expr)
-	}
-	return nil
-}
-
 func createInfSchemaRoute(ctx *plancontext.PlanningContext, table *QueryTable) (*Route, error) {
 	ks, err := ctx.VSchema.AnyKeyspace()
 	if err != nil {
diff --git a/go/vt/vtgate/planbuilder/select.go b/go/vt/vtgate/planbuilder/select.go
index 77ecd41be0..4017662b81 100644
--- a/go/vt/vtgate/planbuilder/select.go
+++ b/go/vt/vtgate/planbuilder/select.go
@@ -66,7 +66,7 @@ func buildSelectPlan(query string) stmtPlanner {
 			return nil, err
 		}
 
-		if shouldRetryWithCNFRewriting(plan) {
+		if shouldRetryAfterPredicateRewriting(plan) {
 			// by transforming the predicates to CNF, the planner will sometimes find better plans
 			primitive := rewriteToCNFAndReplan(stmt, getPlan)
 			if primitive != nil {
@@ -89,12 +89,12 @@ func buildSelectPlan(query string) stmtPlanner {
 }
 
 func rewriteToCNFAndReplan(stmt sqlparser.Statement, getPlan func(sel *sqlparser.Select) (logicalPlan, error)) engine.Primitive {
-	rewritten := sqlparser.RewriteToCNF(stmt)
+	rewritten := sqlparser.RewritePredicate(stmt)
 	sel2, isSelect := rewritten.(*sqlparser.Select)
 	if isSelect {
 		log.Infof("retrying plan after cnf: %s", sqlparser.String(sel2))
 		plan2, err := getPlan(sel2)
-		if err == nil && !shouldRetryWithCNFRewriting(plan2) {
+		if err == nil && !shouldRetryAfterPredicateRewriting(plan2) {
 			// we only use this new plan if it's better than the old one we got
 			return plan2.Primitive()
 		}
@@ -102,7 +102,7 @@ func rewriteToCNFAndReplan(stmt sqlparser.Statement, getPlan func(sel *sqlparser
 	return nil
 }
 
-func shouldRetryWithCNFRewriting(plan logicalPlan) bool {
+func shouldRetryAfterPredicateRewriting(plan logicalPlan) bool {
 	// if we have a I_S query, but have not found table_schema or table_name, let's try CNF
 	var opcode engine.Opcode
 	var sysTableTableName map[string]evalengine.Expr


Test patch:
diff --git a/go/test/endtoend/vtgate/queries/misc/misc_test.go b/go/test/endtoend/vtgate/queries/misc/misc_test.go
index ff6798c6af..16ef7428ab 100644
--- a/go/test/endtoend/vtgate/queries/misc/misc_test.go
+++ b/go/test/endtoend/vtgate/queries/misc/misc_test.go
@@ -131,7 +131,8 @@ func TestQueryTimeoutWithTables(t *testing.T) {
 	// the query usually takes more than 5ms to return. So this should fail.
 	_, err := utils.ExecAllowError(t, mcmp.VtConn, "select /*vt+ PLANNER=gen4 QUERY_TIMEOUT_MS=1 */ count(*) from uks.unsharded where id1 > 31")
 	require.Error(t, err)
-	assert.Contains(t, err.Error(), "context deadline exceeded (errno 1317) (sqlstate 70100)")
+	assert.Contains(t, err.Error(), "context deadline exceeded")
+	assert.Contains(t, err.Error(), "(errno 1317) (sqlstate 70100)")
 
 	// sharded
 	for i := 0; i < 300000; i += 1000 {
@@ -152,5 +153,6 @@ func TestQueryTimeoutWithTables(t *testing.T) {
 	// the query usually takes more than 5ms to return. So this should fail.
 	_, err = utils.ExecAllowError(t, mcmp.VtConn, "select /*vt+ PLANNER=gen4 QUERY_TIMEOUT_MS=1 */ count(*) from t1 where id1 > 31")
 	require.Error(t, err)
-	assert.Contains(t, err.Error(), "context deadline exceeded (errno 1317) (sqlstate 70100)")
+	assert.Contains(t, err.Error(), "context deadline exceeded")
+	assert.Contains(t, err.Error(), "(errno 1317) (sqlstate 70100)")
 }
diff --git a/go/vt/sqlparser/ast_rewriting_test.go b/go/vt/sqlparser/ast_rewriting_test.go
index 7b622e6600..5c8a348881 100644
--- a/go/vt/sqlparser/ast_rewriting_test.go
+++ b/go/vt/sqlparser/ast_rewriting_test.go
@@ -511,101 +511,6 @@ func TestRewritesWithDefaultKeyspace(in *testing.T) {
 	}
 }
 
-func TestRewriteToCNF(in *testing.T) {
-	tests := []struct {
-		in       string
-		expected string
-	}{{
-		in:       "not (not A = 3)",
-		expected: "A = 3",
-	}, {
-		in:       "not (A = 3 and B = 2)",
-		expected: "not A = 3 or not B = 2",
-	}, {
-		in:       "not (A = 3 or B = 2)",
-		expected: "not A = 3 and not B = 2",
-	}, {
-		in:       "A xor B",
-		expected: "(A or B) and not (A and B)",
-	}, {
-		in:       "(A and B) or C",
-		expected: "(A or C) and (B or C)",
-	}, {
-		in:       "C or (A and B)",
-		expected: "(C or A) and (C or B)",
-	}, {
-		in:       "A and A",
-		expected: "A",
-	}, {
-		in:       "A OR A",
-		expected: "A",
-	}, {
-		in:       "A OR (A AND B)",
-		expected: "A",
-	}, {
-		in:       "A OR (B AND A)",
-		expected: "A",
-	}, {
-		in:       "(A AND B) OR A",
-		expected: "A",
-	}, {
-		in:       "(B AND A) OR A",
-		expected: "A",
-	}, {
-		in:       "(A and B) and (B and A)",
-		expected: "A and B",
-	}, {
-		in:       "(A or B) and A",
-		expected: "A",
-	}, {
-		in:       "A and (A or B)",
-		expected: "A",
-	}}
-
-	for _, tc := range tests {
-		in.Run(tc.in, func(t *testing.T) {
-			stmt, err := Parse("SELECT * FROM T WHERE " + tc.in)
-			require.NoError(t, err)
-
-			expr := stmt.(*Select).Where.Expr
-			expr, didRewrite := rewriteToCNFExpr(expr)
-			assert.True(t, didRewrite)
-			assert.Equal(t, tc.expected, String(expr))
-		})
-	}
-}
-
-func TestFixedPointRewriteToCNF(in *testing.T) {
-	tests := []struct {
-		in       string
-		expected string
-	}{{
-		in:       "A xor B",
-		expected: "(A or B) and (not A or not B)",
-	}, {
-		in:       "(A and B) and (B and A) and (B and A) and (A and B)",
-		expected: "A and B",
-	}, {
-		in:       "((A and B) OR (A and C) OR (A and D)) and E and F",
-		expected: "A and ((A or B) and (B or C or A)) and ((A or D) and ((B or A or D) and (B or C or D))) and E and F",
-	}, {
-		in:       "(A and B) OR (A and C)",
-		expected: "A and ((B or A) and (B or C))",
-	}}
-
-	for _, tc := range tests {
-		in.Run(tc.in, func(t *testing.T) {
-			require := require.New(t)
-			stmt, err := Parse("SELECT * FROM T WHERE " + tc.in)
-			require.NoError(err)
-
-			expr := stmt.(*Select).Where.Expr
-			output := RewriteToCNF(expr)
-			assert.Equal(t, tc.expected, String(output))
-		})
-	}
-}
-
 func TestReservedVars(t *testing.T) {
 	for _, prefix := range []string{"vtg", "bv"} {
 		t.Run("prefix_"+prefix, func(t *testing.T) {
diff --git a/go/vt/sqlparser/predicate_rewriting_test.go b/go/vt/sqlparser/predicate_rewriting_test.go
new file mode 100644
index 0000000000..dc60cb04f2
--- /dev/null
+++ b/go/vt/sqlparser/predicate_rewriting_test.go
@@ -0,0 +1,134 @@
+/*
+Copyright 2022 The Vitess Authors.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package sqlparser
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+)
+
+func TestSimplifyExpression(in *testing.T) {
+	tests := []struct {
+		in       string
+		expected string
+	}{{
+		in:       "not (not A = 3)",
+		expected: "A = 3",
+	}, {
+		in:       "not (A = 3 and B = 2)",
+		expected: "not A = 3 or not B = 2",
+	}, {
+		in:       "not (A = 3 or B = 2)",
+		expected: "not A = 3 and not B = 2",
+	}, {
+		in:       "A xor B",
+		expected: "(A or B) and not (A and B)",
+	}, {
+		in:       "(A and B) or C",
+		expected: "(A or C) and (B or C)",
+	}, {
+		in:       "C or (A and B)",
+		expected: "(C or A) and (C or B)",
+	}, {
+		in:       "A and A",
+		expected: "A",
+	}, {
+		in:       "A OR A",
+		expected: "A",
+	}, {
+		in:       "A OR (A AND B)",
+		expected: "A",
+	}, {
+		in:       "A OR (B AND A)",
+		expected: "A",
+	}, {
+		in:       "(A AND B) OR A",
+		expected: "A",
+	}, {
+		in:       "(B AND A) OR A",
+		expected: "A",
+	}, {
+		in:       "(A and B) and (B and A)",
+		expected: "A and B",
+	}, {
+		in:       "(A or B) and A",
+		expected: "A",
+	}, {
+		in:       "A and (A or B)",
+		expected: "A",
+	}, {
+		in:       "(A and B) OR (A and C)",
+		expected: "A and (B or C)",
+	}, {
+		in:       "(A and B) OR (C and A)",
+		expected: "A and (B or C)",
+	}, {
+		in:       "(B and A) OR (A and C)",
+		expected: "A and (B or C)",
+	}, {
+		in:       "(B and A) OR (C and A)",
+		expected: "A and (B or C)",
+	}}
+
+	for _, tc := range tests {
+		in.Run(tc.in, func(t *testing.T) {
+			expr, err := ParseExpr(tc.in)
+			require.NoError(t, err)
+
+			expr, didRewrite := simplifyExpression(expr)
+			assert.True(t, didRewrite == Changed)
+			assert.Equal(t, tc.expected, String(expr))
+		})
+	}
+}
+
+func TestRewritePredicate(in *testing.T) {
+	tests := []struct {
+		in       string
+		expected string
+	}{{
+		in:       "A xor B",
+		expected: "(A or B) and (not A or not B)",
+	}, {
+		in:       "(A and B) and (B and A) and (B and A) and (A and B)",
+		expected: "A and B",
+	}, {
+		in:       "((A and B) OR (A and C) OR (A and D)) and E and F",
+		expected: "A and (B or C or D) and E and F",
+	}, {
+		in:       "(A and B) OR (A and C)",
+		expected: "A and (B or C)",
+	}, {
+		in:       "(A and B) or (A and C) or (A and D)",
+		expected: "A and (B or C or D)",
+	}, {
+		in:       "(a=1 or a IN (1,2)) or (a = 2 or a = 3)",
+		expected: "a in (1, 2, 3)",
+	}}
+
+	for _, tc := range tests {
+		in.Run(tc.in, func(t *testing.T) {
+			expr, err := ParseExpr(tc.in)
+			require.NoError(t, err)
+
+			output := RewritePredicate(expr)
+			assert.Equal(t, tc.expected, String(output))
+		})
+	}
+}
diff --git a/go/vt/vtgate/planbuilder/testdata/ddl_cases.json b/go/vt/vtgate/planbuilder/testdata/ddl_cases.json
index 25d8fd23e8..cca249bcf0 100644
--- a/go/vt/vtgate/planbuilder/testdata/ddl_cases.json
+++ b/go/vt/vtgate/planbuilder/testdata/ddl_cases.json
@@ -513,4 +513,4 @@
       ]
     }
   }
-]
\ No newline at end of file
+]
diff --git a/go/vt/vtgate/planbuilder/testdata/ddl_cases_no_default_keyspace.json b/go/vt/vtgate/planbuilder/testdata/ddl_cases_no_default_keyspace.json
index fca52d487a..95609166fb 100644
--- a/go/vt/vtgate/planbuilder/testdata/ddl_cases_no_default_keyspace.json
+++ b/go/vt/vtgate/planbuilder/testdata/ddl_cases_no_default_keyspace.json
@@ -786,4 +786,4 @@
     "query": "rename table user_extra to main.b",
     "plan": "Changing schema from 'user' to 'main' is not allowed"
   }
-]
\ No newline at end of file
+]
diff --git a/go/vt/vtgate/planbuilder/testdata/filter_cases.json b/go/vt/vtgate/planbuilder/testdata/filter_cases.json
index c4d1c605aa..556e945c20 100644
--- a/go/vt/vtgate/planbuilder/testdata/filter_cases.json
+++ b/go/vt/vtgate/planbuilder/testdata/filter_cases.json
@@ -2234,14 +2234,18 @@
       "Original": "select id from user where user.id = 1 or user.name = 'aa' and user.id in (1, 2)",
       "Instructions": {
         "OperatorType": "Route",
-        "Variant": "Scatter",
+        "Variant": "IN",
         "Keyspace": {
           "Name": "user",
           "Sharded": true
         },
         "FieldQuery": "select id from `user` where 1 != 1",
-        "Query": "select id from `user` where `user`.id = 1 or `user`.`name` = 'aa' and `user`.id in (1, 2)",
-        "Table": "`user`"
+        "Query": "select id from `user` where (`user`.id = 1 or `user`.`name` = 'aa') and `user`.id in ::__vals",
+        "Table": "`user`",
+        "Values": [
+          "(INT64(1), INT64(2))"
+        ],
+        "Vindex": "user_index"
       },
       "TablesUsed": [
         "user.user"
@@ -6246,7 +6250,7 @@
           "Sharded": true
         },
         "FieldQuery": "select col from `user` where 1 != 1",
-        "Query": "select col from `user` where id = 1 or id = 2",
+        "Query": "select col from `user` where id in ::__vals",
         "Table": "`user`",
         "Values": [
           "(INT64(1), INT64(2))"
@@ -6287,7 +6291,7 @@
           "Sharded": true
         },
         "FieldQuery": "select col from `user` where 1 != 1",
-        "Query": "select col from `user` where id = 1 or id = 2 or id = 3",
+        "Query": "select col from `user` where id in ::__vals",
         "Table": "`user`",
         "Values": [
           "(INT64(1), INT64(2), INT64(3))"
@@ -6328,7 +6332,7 @@
           "Sharded": true
         },
         "FieldQuery": "select col from `user` where 1 != 1",
-        "Query": "select col from `user` where id = 1 or id = 2 or (id = 3 or id = 4)",
+        "Query": "select col from `user` where id in ::__vals",
         "Table": "`user`",
         "Values": [
           "(INT64(1), INT64(2), INT64(3), INT64(4))"
@@ -6495,5 +6499,46 @@
         "main.unsharded"
       ]
     }
+  },
+  {
+    "comment": "Single table unique vindex route hiding behind a silly OR",
+    "query": "select id from user where (id = 5 and name ='apa') or (id = 5 and foo = 'bar')",
+    "v3-plan": {
+      "QueryType": "SELECT",
+      "Original": "select id from user where (id = 5 and name ='apa') or (id = 5 and foo = 'bar')",
+      "Instructions": {
+        "OperatorType": "Route",
+        "Variant": "Scatter",
+        "Keyspace": {
+          "Name": "user",
+          "Sharded": true
+        },
+        "FieldQuery": "select id from `user` where 1 != 1",
+        "Query": "select id from `user` where id = 5 and `name` = 'apa' or id = 5 and foo = 'bar'",
+        "Table": "`user`"
+      }
+    },
+    "gen4-plan": {
+      "QueryType": "SELECT",
+      "Original": "select id from user where (id = 5 and name ='apa') or (id = 5 and foo = 'bar')",
+      "Instructions": {
+        "OperatorType": "Route",
+        "Variant": "EqualUnique",
+        "Keyspace": {
+          "Name": "user",
+          "Sharded": true
+        },
+        "FieldQuery": "select id from `user` where 1 != 1",
+        "Query": "select id from `user` where id = 5 and (`name` = 'apa' or foo = 'bar')",
+        "Table": "`user`",
+        "Values": [
+          "INT64(5)"
+        ],
+        "Vindex": "user_index"
+      },
+      "TablesUsed": [
+        "user.user"
+      ]
+    }
   }
 ]
diff --git a/go/vt/vtgate/planbuilder/testdata/systemtables_cases.json b/go/vt/vtgate/planbuilder/testdata/systemtables_cases.json
index a880fec7fd..45258f7d96 100644
--- a/go/vt/vtgate/planbuilder/testdata/systemtables_cases.json
+++ b/go/vt/vtgate/planbuilder/testdata/systemtables_cases.json
@@ -860,7 +860,7 @@
           "Sharded": false
         },
         "FieldQuery": "select * from INFORMATION_SCHEMA.`TABLES` where 1 != 1",
-        "Query": "select * from INFORMATION_SCHEMA.`TABLES` where TABLE_SCHEMA = :__vtschemaname and (other_column = 42 or TABLE_SCHEMA = 'ks') and (other_column = 42 or foobar = 'value')",
+        "Query": "select * from INFORMATION_SCHEMA.`TABLES` where TABLE_SCHEMA = :__vtschemaname and (other_column = 42 or foobar = 'value')",
         "SysTableTableSchema": "[VARCHAR(\"ks\")]",
         "Table": "INFORMATION_SCHEMA.`TABLES`"
       }
@@ -1453,4 +1453,4 @@
       }
     }
   }
-]
\ No newline at end of file
+]