From b3c2cbc072ae80fef697e9167931f656117b8bd1 Mon Sep 17 00:00:00 2001 From: FlorentP <35779988+frouioui@users.noreply.github.com> Date: Thu, 8 Dec 2022 15:52:52 +0100 Subject: [PATCH] Simplify `getPlan` and `gen4CompareV3` (#11903) * simplify getPlan and gen4CompareV3 Signed-off-by: Florent Poinsard * fix error return in getPlan Signed-off-by: Florent Poinsard Signed-off-by: Florent Poinsard --- go/vt/vtgate/engine/compare_utils.go | 73 +++++++++++++++++++ go/vt/vtgate/engine/gen4_compare_v3.go | 55 +------------- go/vt/vtgate/executor.go | 4 + .../planbuilder/gen4_compare_v3_planner.go | 2 +- 4 files changed, 81 insertions(+), 53 deletions(-) create mode 100644 go/vt/vtgate/engine/compare_utils.go diff --git a/go/vt/vtgate/engine/compare_utils.go b/go/vt/vtgate/engine/compare_utils.go new file mode 100644 index 00000000000..c854d6723d3 --- /dev/null +++ b/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 +} diff --git a/go/vt/vtgate/engine/gen4_compare_v3.go b/go/vt/vtgate/engine/gen4_compare_v3.go index c69fa9670af..a913c442a2c 100644 --- a/go/vt/vtgate/engine/gen4_compare_v3.go +++ b/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 -} diff --git a/go/vt/vtgate/executor.go b/go/vt/vtgate/executor.go index 53e7a74e0ae..6b7428aa39f 100644 --- a/go/vt/vtgate/executor.go +++ b/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{':'}) diff --git a/go/vt/vtgate/planbuilder/gen4_compare_v3_planner.go b/go/vt/vtgate/planbuilder/gen4_compare_v3_planner.go index 53dcb8d3153..28879258dd0 100644 --- a/go/vt/vtgate/planbuilder/gen4_compare_v3_planner.go +++ b/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 }