change semantics and planner to handle the new AndExpr

Signed-off-by: Andres Taylor <[email protected]>
vitessio · Aug 28, 2024 · cf7430e · cf7430e
1 parent de1262e
commit cf7430e
Show file tree

Hide file tree

Showing 17 changed files with 122 additions and 104 deletions.
diff --git a/go/vt/sqlparser/ast.go b/go/vt/sqlparser/ast.go
@@ -2263,7 +2263,7 @@ type (
 	}
 
 	// AndExpr represents an AND expression.
-	AndExpr struct{ Predicates []Expr }
+	AndExpr struct{ Predicates Exprs }
 
 	// OrExpr represents an OR expression.
 	OrExpr struct {

diff --git a/go/vt/sqlparser/ast_clone.go b/go/vt/sqlparser/ast_clone.go
diff --git a/go/vt/sqlparser/ast_copy_on_rewrite.go b/go/vt/sqlparser/ast_copy_on_rewrite.go
diff --git a/go/vt/sqlparser/ast_equals.go b/go/vt/sqlparser/ast_equals.go
diff --git a/go/vt/sqlparser/ast_funcs.go b/go/vt/sqlparser/ast_funcs.go
@@ -2360,6 +2360,17 @@ func SplitAndExpression(filters []Expr, node Expr) []Expr {
 	return append(filters, node)
 }
 
+func CreateAndExpr(exprs ...Expr) Expr {
+	switch len(exprs) {
+	case 0:
+		return nil
+	case 1:
+		return exprs[0]
+	default:
+		return &AndExpr{Predicates: exprs}
+	}
+}
+
 // AndExpressions ands together two or more expressions, minimising the expr when possible
 func AndExpressions(exprs ...Expr) Expr {
 	switch len(exprs) {

diff --git a/go/vt/sqlparser/ast_rewrite.go b/go/vt/sqlparser/ast_rewrite.go
diff --git a/go/vt/sqlparser/ast_visit.go b/go/vt/sqlparser/ast_visit.go
diff --git a/go/vt/sqlparser/predicate_rewriting.go b/go/vt/sqlparser/predicate_rewriting.go
@@ -181,10 +181,13 @@ func simplifyOr(or *OrExpr) (Expr, bool) {
 	// Distribution Law
 	var distributedPredicates []Expr
 	for _, lp := range and.Predicates {
-		distributedPredicates = append(distributedPredicates, &OrExpr{
-			Left:  lp,
-			Right: other,
-		})
+		var or *OrExpr
+		if lok {
+			or = &OrExpr{Left: lp, Right: other}
+		} else {
+			or = &OrExpr{Left: other, Right: lp}
+		}
+		distributedPredicates = append(distributedPredicates, or)
 	}
 	return AndExpressions(distributedPredicates...), true
 }

diff --git a/go/vt/vtgate/evalengine/translate.go b/go/vt/vtgate/evalengine/translate.go
@@ -99,15 +99,35 @@ func (ast *astCompiler) translateLogicalNot(node *sqlparser.NotExpr) (IR, error)
 	return &NotExpr{UnaryExpr{inner}}, nil
 }
 
+func (ast *astCompiler) translateLogicalAnd(node *sqlparser.AndExpr) (IR, error) {
+	var acc IR
+	for i, pred := range node.Predicates {
+		ir, err := ast.translateExpr(pred)
+		if err != nil {
+			return nil, err
+		}
+		if i == 0 {
+			acc = ir
+			continue
+		}
+
+		acc = &LogicalExpr{
+			BinaryExpr: BinaryExpr{
+				Left:  acc,
+				Right: ir,
+			},
+			op: opLogicalAnd{},
+		}
+	}
+
+	return acc, nil
+}
+
 func (ast *astCompiler) translateLogicalExpr(node sqlparser.Expr) (IR, error) {
 	var left, right sqlparser.Expr
 
 	var logic opLogical
 	switch n := node.(type) {
-	case *sqlparser.AndExpr:
-		left = n.Left
-		right = n.Right
-		logic = opLogicalAnd{}
 	case *sqlparser.OrExpr:
 		left = n.Left
 		right = n.Right
@@ -521,7 +541,7 @@ func (ast *astCompiler) translateExpr(e sqlparser.Expr) (IR, error) {
 	case *sqlparser.Literal:
 		return translateLiteral(node, ast.cfg.Collation)
 	case *sqlparser.AndExpr:
-		return ast.translateLogicalExpr(node)
+		return ast.translateLogicalAnd(node)
 	case *sqlparser.OrExpr:
 		return ast.translateLogicalExpr(node)
 	case *sqlparser.XorExpr:

diff --git a/go/vt/vtgate/planbuilder/operators/querygraph.go b/go/vt/vtgate/planbuilder/operators/querygraph.go
@@ -141,10 +141,7 @@ func (qg *QueryGraph) addNoDepsPredicate(predicate sqlparser.Expr) {
 	if qg.NoDeps == nil {
 		qg.NoDeps = predicate
 	} else {
-		qg.NoDeps = &sqlparser.AndExpr{
-			Left:  qg.NoDeps,
-			Right: predicate,
-		}
+		qg.NoDeps = sqlparser.AndExpressions(qg.NoDeps, predicate)
 	}
 }
 

diff --git a/go/vt/vtgate/planbuilder/operators/subquery.go b/go/vt/vtgate/planbuilder/operators/subquery.go
@@ -298,7 +298,7 @@ func (sq *SubQuery) settleFilter(ctx *plancontext.PlanningContext, outer Operato
 		// lead to better routing. This however might not always be true for example we can have the rhsPred to be something like
 		// `user.id = 2 OR (:__sq_has_values AND user.id IN ::sql1)`
 		if andExpr, isAndExpr := rhsPred.(*sqlparser.AndExpr); isAndExpr {
-			predicates = append(predicates, andExpr.Left, andExpr.Right)
+			predicates = append(predicates, andExpr.Predicates...)
 		} else {
 			predicates = append(predicates, rhsPred)
 		}

diff --git a/go/vt/vtgate/planbuilder/operators/update.go b/go/vt/vtgate/planbuilder/operators/update.go
@@ -743,10 +743,7 @@ func buildChildUpdOpForSetNull(
 		updatedTable,
 		updateExprs, fk, updatedTable.GetTableName(), nonLiteralUpdateInfo, false /* appendQualifier */)
 	if compExpr != nil {
-		childWhereExpr = &sqlparser.AndExpr{
-			Left:  childWhereExpr,
-			Right: compExpr,
-		}
+		childWhereExpr = sqlparser.AndExpressions(childWhereExpr, compExpr)
 	}
 	parsedComments := getParsedCommentsForFkChecks(ctx)
 	childUpdStmt := &sqlparser.Update{
@@ -847,13 +844,12 @@ func createFkVerifyOpForParentFKForUpdate(ctx *plancontext.PlanningContext, upda
 		var predicate sqlparser.Expr = parentIsNullExpr
 		var joinExpr sqlparser.Expr
 		if matchedExpr == nil {
-			predicate = &sqlparser.AndExpr{
-				Left: predicate,
-				Right: &sqlparser.IsExpr{
+			predicate = sqlparser.AndExpressions(
+				predicate,
+				&sqlparser.IsExpr{
 					Left:  sqlparser.NewColNameWithQualifier(pFK.ChildColumns[idx].String(), childTbl),
 					Right: sqlparser.IsNotNullOp,
-				},
-			}
+				})
 			joinExpr = &sqlparser.ComparisonExpr{
 				Operator: sqlparser.EqualOp,
 				Left:     sqlparser.NewColNameWithQualifier(pFK.ParentColumns[idx].String(), parentTbl),
@@ -868,35 +864,33 @@ func createFkVerifyOpForParentFKForUpdate(ctx *plancontext.PlanningContext, upda
 				Left:     sqlparser.NewColNameWithQualifier(pFK.ParentColumns[idx].String(), parentTbl),
 				Right:    prefixedMatchExpr,
 			}
-			predicate = &sqlparser.AndExpr{
-				Left: predicate,
-				Right: &sqlparser.IsExpr{
+			predicate = sqlparser.AndExpressions(
+				predicate,
+				&sqlparser.IsExpr{
 					Left:  prefixedMatchExpr,
 					Right: sqlparser.IsNotNullOp,
-				},
-			}
+				})
 		}
 
 		if idx == 0 {
 			joinCond, whereCond = joinExpr, predicate
 			continue
 		}
-		joinCond = &sqlparser.AndExpr{Left: joinCond, Right: joinExpr}
-		whereCond = &sqlparser.AndExpr{Left: whereCond, Right: predicate}
+		joinCond = sqlparser.AndExpressions(joinCond, joinExpr)
+		whereCond = sqlparser.AndExpressions(whereCond, predicate)
 	}
-	whereCond = &sqlparser.AndExpr{
-		Left: whereCond,
-		Right: &sqlparser.NotExpr{
+	whereCond = sqlparser.AndExpressions(
+		whereCond,
+		&sqlparser.NotExpr{
 			Expr: &sqlparser.ComparisonExpr{
 				Operator: sqlparser.NullSafeEqualOp,
 				Left:     notEqualColNames,
 				Right:    notEqualExprs,
 			},
-		},
-	}
+		})
 	// add existing where condition on the update statement
 	if updStmt.Where != nil {
-		whereCond = &sqlparser.AndExpr{Left: whereCond, Right: prefixColNames(ctx, childTbl, updStmt.Where.Expr)}
+		whereCond = sqlparser.AndExpressions(whereCond, prefixColNames(ctx, childTbl, updStmt.Where.Expr))
 	}
 	return createSelectionOp(ctx,
 		sqlparser.SelectExprs{sqlparser.NewAliasedExpr(sqlparser.NewIntLiteral("1"), "")},
@@ -959,7 +953,7 @@ func createFkVerifyOpForChildFKForUpdate(ctx *plancontext.PlanningContext, updat
 			joinCond = joinExpr
 			continue
 		}
-		joinCond = &sqlparser.AndExpr{Left: joinCond, Right: joinExpr}
+		joinCond = sqlparser.AndExpressions(joinCond, joinExpr)
 	}
 
 	var whereCond sqlparser.Expr

diff --git a/go/vt/vtgate/planbuilder/predicate_rewrite_test.go b/go/vt/vtgate/planbuilder/predicate_rewrite_test.go
@@ -64,10 +64,10 @@ func (tc testCase) createPredicate(lvl int) sqlparser.Expr {
 			Expr: tc.createPredicate(lvl + 1),
 		}
 	case AND:
-		return &sqlparser.AndExpr{
-			Left:  tc.createPredicate(lvl + 1),
-			Right: tc.createPredicate(lvl + 1),
-		}
+		return sqlparser.AndExpressions(
+			tc.createPredicate(lvl+1),
+			tc.createPredicate(lvl+1),
+		)
 	case OR:
 		return &sqlparser.OrExpr{
 			Left:  tc.createPredicate(lvl + 1),