Simplify getPlan and gen4CompareV3 (vitessio#11903)

* simplify getPlan and gen4CompareV3

Signed-off-by: Florent Poinsard <[email protected]>

* fix error return in getPlan

Signed-off-by: Florent Poinsard <[email protected]>

Signed-off-by: Florent Poinsard <[email protected]>

go/vt/vtgate/engine/compare_utils.go

@@ -0,0 +1,73 @@
+/*
+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 engine
+
+import (
+	"encoding/json"
+
+	"vitess.io/vitess/go/sqltypes"
+	"vitess.io/vitess/go/vt/log"
+	vtrpcpb "vitess.io/vitess/go/vt/proto/vtrpc"
+	"vitess.io/vitess/go/vt/vterrors"
+)
+
+func printMismatch(leftResult, rightResult *sqltypes.Result, leftPrimitive, rightPrimitive Primitive, leftName, rightName string) {
+	log.Errorf("Results of %s and %s are not equal. Displaying diff.", rightName, leftName)
+
+	// get right plan and print it
+	rightplan := &Plan{
+		Instructions: rightPrimitive,
+	}
+	rightJSON, _ := json.MarshalIndent(rightplan, "", "  ")
+	log.Errorf("%s's plan:\n%s", rightName, string(rightJSON))
+
+	// get left's plan and print it
+	leftplan := &Plan{
+		Instructions: leftPrimitive,
+	}
+	leftJSON, _ := json.MarshalIndent(leftplan, "", "  ")
+	log.Errorf("%s's plan:\n%s", leftName, string(leftJSON))
+
+	log.Errorf("%s's results:\n", rightName)
+	log.Errorf("\t[rows affected: %d]\n", rightResult.RowsAffected)
+	for _, row := range rightResult.Rows {
+		log.Errorf("\t%s", row)
+	}
+	log.Errorf("%s's results:\n", leftName)
+	log.Errorf("\t[rows affected: %d]\n", leftResult.RowsAffected)
+	for _, row := range leftResult.Rows {
+		log.Errorf("\t%s", row)
+	}
+	log.Error("End of diff.")
+}
+
+// CompareErrors compares the two errors, and if they don't match, produces an error
+func CompareErrors(leftErr, rightErr error, leftName, rightName string) error {
+	if leftErr != nil && rightErr != nil {
+		if leftErr.Error() == rightErr.Error() {
+			return rightErr
+		}
+		return vterrors.Errorf(vtrpcpb.Code_INTERNAL, "%s and %s failed with different errors: %s: [%s], %s: [%s]", leftName, rightName, leftErr.Error(), rightErr.Error(), leftName, rightName)
+	}
+	if leftErr == nil && rightErr != nil {
+		return vterrors.Errorf(vtrpcpb.Code_INTERNAL, "%s failed while %s did not: %s", rightName, rightErr.Error(), leftName)
+	}
+	if leftErr != nil && rightErr == nil {
+		return vterrors.Errorf(vtrpcpb.Code_INTERNAL, "%s failed while %s did not: %s", leftName, leftErr.Error(), rightName)
+	}
+	return nil
+}

go/vt/vtgate/engine/gen4_compare_v3.go

@@ -18,11 +18,9 @@ package engine
 
 import (
 	"context"
-	"encoding/json"
 	"sync"
 
 	"vitess.io/vitess/go/sqltypes"
-	"vitess.io/vitess/go/vt/log"
 	querypb "vitess.io/vitess/go/vt/proto/query"
 	vtrpcpb "vitess.io/vitess/go/vt/proto/vtrpc"
 	"vitess.io/vitess/go/vt/vterrors"
@@ -78,7 +76,7 @@ func (gc *Gen4CompareV3) TryExecute(ctx context.Context, vcursor VCursor, bindVa
 		v3Result, v3Err = gc.V3.TryExecute(ctx, vcursor, bindVars, wantfields)
 	}
 
-	if err := CompareV3AndGen4Errors(v3Err, gen4Err); err != nil {
+	if err := CompareErrors(v3Err, gen4Err, "v3", "Gen4"); err != nil {
 		return nil, err
 	}
 
@@ -111,7 +109,7 @@ func (gc *Gen4CompareV3) TryStreamExecute(ctx context.Context, vcursor VCursor,
 		})
 	}
 
-	if err := CompareV3AndGen4Errors(v3Err, gen4Err); err != nil {
+	if err := CompareErrors(v3Err, gen4Err, "v3", "Gen4"); err != nil {
 		return err
 	}
 
@@ -129,42 +127,12 @@ func (gc *Gen4CompareV3) compareResults(v3Result *sqltypes.Result, gen4Result *s
 		match = sqltypes.ResultsEqualUnordered([]sqltypes.Result{*v3Result}, []sqltypes.Result{*gen4Result})
 	}
 	if !match {
-		gc.printMismatch(v3Result, gen4Result)
+		printMismatch(v3Result, gen4Result, gc.V3, gc.Gen4, "V3", "Gen4")
 		return vterrors.Errorf(vtrpcpb.Code_INTERNAL, "results did not match, see VTGate's logs for more information")
 	}
 	return nil
 }
 
-func (gc *Gen4CompareV3) printMismatch(v3Result *sqltypes.Result, gen4Result *sqltypes.Result) {
-	log.Warning("Results of Gen4 and V3 are not equal. Displaying diff.")
-
-	// get Gen4 plan and print it
-	gen4plan := &Plan{
-		Instructions: gc.Gen4,
-	}
-	gen4JSON, _ := json.MarshalIndent(gen4plan, "", "  ")
-	log.Warning("Gen4's plan:\n", string(gen4JSON))
-
-	// get V3's plan and print it
-	v3plan := &Plan{
-		Instructions: gc.V3,
-	}
-	v3JSON, _ := json.MarshalIndent(v3plan, "", "  ")
-	log.Warning("V3's plan:\n", string(v3JSON))
-
-	log.Warning("Gen4's results:\n")
-	log.Warningf("\t[rows affected: %d]\n", gen4Result.RowsAffected)
-	for _, row := range gen4Result.Rows {
-		log.Warningf("\t%s", row)
-	}
-	log.Warning("V3's results:\n")
-	log.Warningf("\t[rows affected: %d]\n", v3Result.RowsAffected)
-	for _, row := range v3Result.Rows {
-		log.Warningf("\t%s", row)
-	}
-	log.Warning("End of diff.")
-}
-
 // Inputs implements the Primitive interface
 func (gc *Gen4CompareV3) Inputs() []Primitive {
 	return []Primitive{gc.Gen4, gc.V3}
@@ -174,20 +142,3 @@ func (gc *Gen4CompareV3) Inputs() []Primitive {
 func (gc *Gen4CompareV3) description() PrimitiveDescription {
 	return PrimitiveDescription{OperatorType: "Gen4CompareV3"}
 }
-
-// CompareV3AndGen4Errors compares the two errors, and if they don't match, produces an error
-func CompareV3AndGen4Errors(v3Err, gen4Err error) error {
-	if v3Err != nil && gen4Err != nil {
-		if v3Err.Error() == gen4Err.Error() {
-			return gen4Err
-		}
-		return vterrors.Errorf(vtrpcpb.Code_INTERNAL, "v3 and Gen4 failed with different errors: v3: [%s], Gen4: [%s]", v3Err.Error(), gen4Err.Error())
-	}
-	if v3Err == nil && gen4Err != nil {
-		return vterrors.Errorf(vtrpcpb.Code_INTERNAL, "Gen4 failed while v3 did not: %s", gen4Err.Error())
-	}
-	if v3Err != nil && gen4Err == nil {
-		return vterrors.Errorf(vtrpcpb.Code_INTERNAL, "v3 failed while Gen4 did not: %s", v3Err.Error())
-	}
-	return nil
-}

go/vt/vtgate/executor.go

@@ -1019,6 +1019,10 @@ func (e *Executor) getPlan(ctx context.Context, vcursor *vcursorImpl, sql string
 	logStats.SQL = comments.Leading + query + comments.Trailing
 	logStats.BindVariables = sqltypes.CopyBindVariables(bindVars)
 
+	return e.cacheAndBuildStatement(ctx, vcursor, query, statement, qo, logStats, stmt, reservedVars, bindVarNeeds)
+}
+
+func (e *Executor) cacheAndBuildStatement(ctx context.Context, vcursor *vcursorImpl, query string, statement sqlparser.Statement, qo iQueryOption, logStats *logstats.LogStats, stmt sqlparser.Statement, reservedVars *sqlparser.ReservedVars, bindVarNeeds *sqlparser.BindVarNeeds) (*engine.Plan, sqlparser.Statement, error) {
 	planHash := sha256.New()
 	_, _ = planHash.Write([]byte(vcursor.planPrefixKey(ctx)))
 	_, _ = planHash.Write([]byte{':'})

go/vt/vtgate/planbuilder/gen4_compare_v3_planner.go

@@ -58,7 +58,7 @@ func gen4CompareV3Planner(query string) func(sqlparser.Statement, *sqlparser.Res
 		v3Primitive, v3Err := planWithPlannerVersion(statement, vars, ctxVSchema, query, V3)
 
 		// check potential errors from Gen4 and V3
-		err = engine.CompareV3AndGen4Errors(v3Err, gen4Err)
+		err = engine.CompareErrors(v3Err, gen4Err, "v3", "Gen4")
 		if err != nil {
 			return nil, err
 		}