Repository: vitessio/vitess
Pull request #11707 (2022-11-15 10:41:54)

endtoend: fix race when closing vtgate
## Description

This fixes a race we've found that can sometimes get the endtoend tests stuck permanently during shutdown. cc @dbussink 

## Related Issue(s)

<!-- List related issues and pull requests. If this PR fixes an issue, please add it using Fixes #????  -->

## Checklist

-   [x] "Backport to:" labels have been added if this change should be back-ported
-   [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. -->


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/.github/CODEOWNERS b/.github/CODEOWNERS
index 7405fe7f21..774db8643b 100644
--- a/.github/CODEOWNERS
+++ b/.github/CODEOWNERS
@@ -1,5 +1,7 @@
 * @deepthi
 bootstrap.sh @ajm188 @deepthi @frouioui @vmg
+go.mod @ajm188 @deepthi @harshit-gangal @mattlord @rohit-nayak-ps @systay @frouioui
+go.sum @ajm188 @deepthi @harshit-gangal @mattlord @rohit-nayak-ps @systay @frouioui
 /.github/ISSUE_TEMPLATE/ @deepthi @frouioui @mattlord
 /.github/workflows/ @deepthi @frouioui @mattlord
 /config/mycnf/ @deepthi @shlomi-noach @mattlord
diff --git a/.github/workflows/static_checks_etc.yml b/.github/workflows/static_checks_etc.yml
index 3084991410..7a45a54426 100644
--- a/.github/workflows/static_checks_etc.yml
+++ b/.github/workflows/static_checks_etc.yml
@@ -90,7 +90,8 @@ jobs:
               - 'Makefile'
               - 'bootstrap.sh'
               - '.github/workflows/static_checks_etc.yml'
-
+            ci_config:
+              - 'test/config.json'
 
       - name: Set up Go
         if: steps.skip-workflow.outputs.skip-workflow == 'false' && steps.changes.outputs.go_files == 'true' || steps.changes.outputs.parser_changes == 'true' || steps.changes.outputs.proto_changes == 'true'
@@ -195,3 +196,8 @@ jobs:
         if: steps.skip-workflow.outputs.skip-workflow == 'false' && steps.changes.outputs.proto_changes == 'true'
         run: |
           tools/check_make_proto.sh || exit 1
+
+      - name: Check test/config.json
+        if: steps.skip-workflow.outputs.skip-workflow == 'false' && (steps.changes.outputs.go_files == 'true' || steps.changes.outputs.ci_config == 'true')
+        run: |
+          go run ./go/tools/ci-config/main.go || exit 1
diff --git a/go.mod b/go.mod
index 97122cfee8..aeafac47ac 100644
--- a/go.mod
+++ b/go.mod
@@ -117,6 +117,7 @@ require (
 
 require (
 	github.com/bndr/gotabulate v1.1.2
+	github.com/nsf/jsondiff v0.0.0-20210926074059-1e845ec5d249
 	github.com/openark/golib v0.0.0-20210531070646-355f37940af8
 	golang.org/x/exp v0.0.0-20221023144134-a1e5550cf13e
 )
diff --git a/go.sum b/go.sum
index a0ed58c44d..bcff557d4b 100644
--- a/go.sum
+++ b/go.sum
@@ -128,8 +128,6 @@ github.com/blang/semver v3.5.1+incompatible h1:cQNTCjp13qL8KC3Nbxr/y2Bqb63oX6wdn
 github.com/blang/semver v3.5.1+incompatible/go.mod h1:kRBLl5iJ+tD4TcOOxsy/0fnwebNt5EWlYSAyrTnjyyk=
 github.com/bndr/gotabulate v1.1.2 h1:yC9izuZEphojb9r+KYL4W9IJKO/ceIO8HDwxMA24U4c=
 github.com/bndr/gotabulate v1.1.2/go.mod h1:0+8yUgaPTtLRTjf49E8oju7ojpU11YmXyvq1LbPAb3U=
-github.com/buger/jsonparser v0.0.0-20200322175846-f7e751efca13 h1:+qUNY4VRkEH46bLUwxCyUU+iOGJMQBVibAaYzWiwWcg=
-github.com/buger/jsonparser v0.0.0-20200322175846-f7e751efca13/go.mod h1:tgcrVJ81GPSF0mz+0nu1Xaz0fazGPrmmJfJtxjbHhUQ=
 github.com/buger/jsonparser v1.1.1 h1:2PnMjfWD7wBILjqQbt530v576A/cAbQvEW9gGIpYMUs=
 github.com/buger/jsonparser v1.1.1/go.mod h1:6RYKKt7H4d4+iWqouImQ9R2FZql3VbhNgx27UK13J/0=
 github.com/census-instrumentation/opencensus-proto v0.2.1/go.mod h1:f6KPmirojxKA12rnyqOA5BBL4O983OfeGPqjHWSTneU=
@@ -564,6 +562,8 @@ github.com/mwitkow/go-conntrack v0.0.0-20161129095857-cc309e4a2223/go.mod h1:qRW
 github.com/mwitkow/go-conntrack v0.0.0-20190716064945-2f068394615f/go.mod h1:qRWi+5nqEBWmkhHvq77mSJWrCKwh8bxhgT7d/eI7P4U=
 github.com/mxk/go-flowrate v0.0.0-20140419014527-cca7078d478f/go.mod h1:ZdcZmHo+o7JKHSa8/e818NopupXU1YMK5fe1lsApnBw=
 github.com/ngdinhtoan/glide-cleanup v0.2.0/go.mod h1:UQzsmiDOb8YV3nOsCxK/c9zPpCZVNoHScRE3EO9pVMM=
+github.com/nsf/jsondiff v0.0.0-20210926074059-1e845ec5d249 h1:NHrXEjTNQY7P0Zfx1aMrNhpgxHmow66XQtm0aQLY0AE=
+github.com/nsf/jsondiff v0.0.0-20210926074059-1e845ec5d249/go.mod h1:mpRZBD8SJ55OIICQ3iWH0Yz3cjzA61JdqMLoWXeB2+8=
 github.com/nxadm/tail v1.4.4 h1:DQuhQpB1tVlglWS2hLQ5OV6B5r8aGxSrPc5Qo6uTN78=
 github.com/nxadm/tail v1.4.4/go.mod h1:kenIhsEOeOJmVchQTgglprH7qJGnHDVpk1VPCcaMI8A=
 github.com/olekukonko/tablewriter v0.0.0-20170122224234-a0225b3f23b5/go.mod h1:vsDQFd/mU46D+Z4whnwzcISnGGzXWMclvtLoiIKAKIo=
diff --git a/go/tools/ci-config/main.go b/go/tools/ci-config/main.go
new file mode 100644
index 0000000000..d767b6f4d3
--- /dev/null
+++ b/go/tools/ci-config/main.go
@@ -0,0 +1,74 @@
+/*
+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 main
+
+import (
+	"encoding/json"
+	"fmt"
+	"log"
+	"os"
+	"strings"
+)
+
+type Test struct {
+	Args []string
+}
+
+type Config struct {
+	Tests map[string]*Test
+}
+
+func main() {
+	content, err := os.ReadFile("./test/config.json")
+	if err != nil {
+		log.Fatal(err)
+	}
+
+	tests := &Config{}
+	err = json.Unmarshal(content, tests)
+	if err != nil {
+		log.Fatal(err)
+	}
+
+	var failedConfig []string
+	for name, test := range tests.Tests {
+		if len(test.Args) == 0 {
+			continue
+		}
+		path := test.Args[0]
+		if !strings.HasPrefix(path, "vitess.io/vitess/") {
+			continue
+		}
+		path = path[len("vitess.io/vitess/"):]
+
+		stat, err := os.Stat(path)
+		if err != nil || !stat.IsDir() {
+			failedConfig = append(failedConfig, fmt.Sprintf("%s: %s", name, path))
+			continue
+		}
+	}
+
+	if len(failedConfig) > 0 {
+		fmt.Println("Some packages in test/config.json were not found in the codebase:")
+		for _, failed := range failedConfig {
+			fmt.Println("\t" + failed)
+		}
+		fmt.Println("\nYou must remove them from test/config.json to avoid unnecessary CI load.")
+		os.Exit(1)
+	}
+	fmt.Println("The file: test/config.json is clean.")
+}
diff --git a/go/vt/vtgate/semantics/early_rewriter.go b/go/vt/vtgate/semantics/early_rewriter.go
index edb085e58e..cd36962d74 100644
--- a/go/vt/vtgate/semantics/early_rewriter.go
+++ b/go/vt/vtgate/semantics/early_rewriter.go
@@ -19,6 +19,8 @@ package semantics
 import (
 	"strconv"
 
+	"vitess.io/vitess/go/vt/vtgate/evalengine"
+
 	vtrpcpb "vitess.io/vitess/go/vt/proto/vtrpc"
 	"vitess.io/vitess/go/vt/sqlparser"
 	"vitess.io/vitess/go/vt/vterrors"
@@ -56,6 +58,11 @@ func (r *earlyRewriter) down(cursor *sqlparser.Cursor) error {
 	case sqlparser.OrderBy:
 		r.clause = "order clause"
 		rewriteHavingAndOrderBy(cursor, node)
+	case *sqlparser.OrExpr:
+		newNode := rewriteOrFalse(*node)
+		if newNode != nil {
+			cursor.Replace(newNode)
+		}
 	case sqlparser.GroupBy:
 		r.clause = "group statement"
 
@@ -236,6 +243,37 @@ func realCloneOfColNames(expr sqlparser.Expr, union bool) sqlparser.Expr {
 	}, nil).(sqlparser.Expr)
 }
 
+func rewriteOrFalse(orExpr sqlparser.OrExpr) sqlparser.Expr {
+	// we are looking for the pattern `WHERE c = 1 OR 1 = 0`
+	isFalse := func(subExpr sqlparser.Expr) bool {
+		evalEnginePred, err := evalengine.Translate(subExpr, nil)
+		if err != nil {
+			return false
+		}
+
+		env := evalengine.EmptyExpressionEnv()
+		res, err := env.Evaluate(evalEnginePred)
+		if err != nil {
+			return false
+		}
+
+		boolValue, err := res.Value().ToBool()
+		if err != nil {
+			return false
+		}
+
+		return !boolValue
+	}
+
+	if isFalse(orExpr.Left) {
+		return orExpr.Right
+	} else if isFalse(orExpr.Right) {
+		return orExpr.Left
+	}
+
+	return nil
+}
+
 func rewriteJoinUsing(
 	current *scope,
 	using sqlparser.Columns,


Test patch:
diff --git a/go/test/endtoend/cluster/topo_process.go b/go/test/endtoend/cluster/topo_process.go
index bb5901db9c..7326aa57a5 100644
--- a/go/test/endtoend/cluster/topo_process.go
+++ b/go/test/endtoend/cluster/topo_process.go
@@ -103,6 +103,7 @@ func (topo *TopoProcess) SetupEtcd() (err error) {
 	topo.exit = make(chan error)
 	go func() {
 		topo.exit <- topo.proc.Wait()
+		close(topo.exit)
 	}()
 
 	timeout := time.Now().Add(60 * time.Second)
@@ -227,6 +228,7 @@ func (topo *TopoProcess) SetupConsul(cluster *LocalProcessCluster) (err error) {
 	topo.exit = make(chan error)
 	go func() {
 		topo.exit <- topo.proc.Wait()
+		close(topo.exit)
 	}()
 
 	timeout := time.Now().Add(60 * time.Second)
@@ -289,8 +291,9 @@ func (topo *TopoProcess) TearDown(Cell string, originalVtRoot string, currentRoo
 
 		case <-time.After(10 * time.Second):
 			topo.proc.Process.Kill()
+			err := <-topo.exit
 			topo.proc = nil
-			return <-topo.exit
+			return err
 		}
 	}
 
diff --git a/go/test/endtoend/cluster/vtbackup_process.go b/go/test/endtoend/cluster/vtbackup_process.go
index 73253d3be3..b7beed6793 100644
--- a/go/test/endtoend/cluster/vtbackup_process.go
+++ b/go/test/endtoend/cluster/vtbackup_process.go
@@ -92,6 +92,14 @@ func (vtbackup *VtbackupProcess) Setup() (err error) {
 		return
 	}
 
+	vtbackup.exit = make(chan error)
+	go func() {
+		if vtbackup.proc != nil {
+			vtbackup.exit <- vtbackup.proc.Wait()
+			close(vtbackup.exit)
+		}
+	}()
+
 	return nil
 }
 
@@ -111,8 +119,9 @@ func (vtbackup *VtbackupProcess) TearDown() error {
 
 	case <-time.After(10 * time.Second):
 		vtbackup.proc.Process.Kill()
+		err := <-vtbackup.exit
 		vtbackup.proc = nil
-		return <-vtbackup.exit
+		return err
 	}
 }
 
diff --git a/go/test/endtoend/cluster/vtctld_process.go b/go/test/endtoend/cluster/vtctld_process.go
index 074234fb97..3656b7b3ec 100644
--- a/go/test/endtoend/cluster/vtctld_process.go
+++ b/go/test/endtoend/cluster/vtctld_process.go
@@ -87,6 +87,7 @@ func (vtctld *VtctldProcess) Setup(cell string, extraArgs ...string) (err error)
 	vtctld.exit = make(chan error)
 	go func() {
 		vtctld.exit <- vtctld.proc.Wait()
+		close(vtctld.exit)
 	}()
 
 	timeout := time.Now().Add(60 * time.Second)
@@ -138,8 +139,9 @@ func (vtctld *VtctldProcess) TearDown() error {
 
 	case <-time.After(10 * time.Second):
 		vtctld.proc.Process.Kill()
+		err := <-vtctld.exit
 		vtctld.proc = nil
-		return <-vtctld.exit
+		return err
 	}
 }
 
diff --git a/go/test/endtoend/cluster/vtgate_process.go b/go/test/endtoend/cluster/vtgate_process.go
index 54166f47fa..80e2d4cc4d 100644
--- a/go/test/endtoend/cluster/vtgate_process.go
+++ b/go/test/endtoend/cluster/vtgate_process.go
@@ -138,6 +138,7 @@ func (vtgate *VtgateProcess) Setup() (err error) {
 	go func() {
 		if vtgate.proc != nil {
 			vtgate.exit <- vtgate.proc.Wait()
+			close(vtgate.exit)
 		}
 	}()
 
@@ -236,8 +237,9 @@ func (vtgate *VtgateProcess) TearDown() error {
 
 	case <-time.After(30 * time.Second):
 		vtgate.proc.Process.Kill()
+		err := <-vtgate.exit
 		vtgate.proc = nil
-		return <-vtgate.exit
+		return err
 	}
 }
 
diff --git a/go/test/endtoend/cluster/vtgr_process.go b/go/test/endtoend/cluster/vtgr_process.go
index f2205a9323..1960e46948 100644
--- a/go/test/endtoend/cluster/vtgr_process.go
+++ b/go/test/endtoend/cluster/vtgr_process.go
@@ -77,6 +77,7 @@ func (vtgr *VtgrProcess) Start(alias string) (err error) {
 	go func() {
 		if vtgr.proc != nil {
 			vtgr.exit <- vtgr.proc.Wait()
+			close(vtgr.exit)
 		}
 	}()
 
@@ -97,8 +98,9 @@ func (vtgr *VtgrProcess) TearDown() error {
 		return nil
 
 	case <-time.After(10 * time.Second):
-		_ = vtgr.proc.Process.Kill()
+		vtgr.proc.Process.Kill()
+		err := <-vtgr.exit
 		vtgr.proc = nil
-		return <-vtgr.exit
+		return err
 	}
 }
diff --git a/go/test/endtoend/cluster/vtorc_process.go b/go/test/endtoend/cluster/vtorc_process.go
index 14f475fa48..e0a384bdbc 100644
--- a/go/test/endtoend/cluster/vtorc_process.go
+++ b/go/test/endtoend/cluster/vtorc_process.go
@@ -139,6 +139,7 @@ func (orc *VTOrcProcess) Setup() (err error) {
 	go func() {
 		if orc.proc != nil {
 			orc.exit <- orc.proc.Wait()
+			close(orc.exit)
 		}
 	}()
 
@@ -160,8 +161,9 @@ func (orc *VTOrcProcess) TearDown() error {
 
 	case <-time.After(30 * time.Second):
 		_ = orc.proc.Process.Kill()
+		err := <-orc.exit
 		orc.proc = nil
-		return <-orc.exit
+		return err
 	}
 }
 
diff --git a/go/test/endtoend/cluster/vttablet_process.go b/go/test/endtoend/cluster/vttablet_process.go
index 77e3e738a5..a5eba51f22 100644
--- a/go/test/endtoend/cluster/vttablet_process.go
+++ b/go/test/endtoend/cluster/vttablet_process.go
@@ -145,6 +145,7 @@ func (vttablet *VttabletProcess) Setup() (err error) {
 	go func() {
 		if vttablet.proc != nil {
 			vttablet.exit <- vttablet.proc.Wait()
+			close(vttablet.exit)
 		}
 	}()
 
@@ -399,12 +400,10 @@ func (vttablet *VttabletProcess) TearDownWithTimeout(timeout time.Duration) erro
 		return nil
 
 	case <-time.After(timeout):
-		proc := vttablet.proc
-		if proc != nil {
-			vttablet.proc.Process.Kill()
-			vttablet.proc = nil
-		}
-		return <-vttablet.exit
+		vttablet.proc.Process.Kill()
+		err := <-vttablet.exit
+		vttablet.proc = nil
+		return err
 	}
 }
 
diff --git a/go/test/endtoend/utils/cmp.go b/go/test/endtoend/utils/cmp.go
index 89e51c1e66..a377ed777c 100644
--- a/go/test/endtoend/utils/cmp.go
+++ b/go/test/endtoend/utils/cmp.go
@@ -85,6 +85,39 @@ func (mcmp *MySQLCompare) AssertMatchesAny(query string, expected ...string) {
 	mcmp.t.Errorf("Query: %s (-want +got):\n%v\nGot:%s", query, expected, got)
 }
 
+// AssertMatchesAnyNoCompare ensures the given query produces any one of the expected results.
+// This method does not compare the mysql and vitess results together
+func (mcmp *MySQLCompare) AssertMatchesAnyNoCompare(query string, expected ...string) {
+	mcmp.t.Helper()
+
+	mQr, vQr := mcmp.execNoCompare(query)
+	got := fmt.Sprintf("%v", mQr.Rows)
+	valid := false
+	for _, e := range expected {
+		diff := cmp.Diff(e, got)
+		if diff == "" {
+			valid = true
+			break
+		}
+	}
+	if !valid {
+		mcmp.t.Errorf("MySQL Query: %s (-want +got):\n%v\nGot:%s", query, expected, got)
+	}
+	valid = false
+
+	got = fmt.Sprintf("%v", vQr.Rows)
+	for _, e := range expected {
+		diff := cmp.Diff(e, got)
+		if diff == "" {
+			valid = true
+			break
+		}
+	}
+	if !valid {
+		mcmp.t.Errorf("Vitess Query: %s (-want +got):\n%v\nGot:%s", query, expected, got)
+	}
+}
+
 // AssertContainsError executes the query on both Vitess and MySQL.
 // Both clients need to return an error. The error of Vitess must be matching the given expectation.
 func (mcmp *MySQLCompare) AssertContainsError(query, expected string) {
diff --git a/go/test/endtoend/vtgate/queries/derived/derived_test.go b/go/test/endtoend/vtgate/queries/derived/derived_test.go
index 5da8d8bac9..62601ed528 100644
--- a/go/test/endtoend/vtgate/queries/derived/derived_test.go
+++ b/go/test/endtoend/vtgate/queries/derived/derived_test.go
@@ -30,7 +30,7 @@ func start(t *testing.T) (utils.MySQLCompare, func()) {
 	require.NoError(t, err)
 
 	deleteAll := func() {
-		tables := []string{"music"}
+		tables := []string{"music", "user"}
 		for _, table := range tables {
 			_, _ = mcmp.ExecAndIgnore("delete from " + table)
 		}
@@ -56,6 +56,7 @@ func TestDerivedTableWithOrderByLimit(t *testing.T) {
 }
 
 func TestDerivedAggregationOnRHS(t *testing.T) {
+	t.Skip("skipped for now, issue: https://github.com/vitessio/vitess/issues/11703")
 	mcmp, closer := start(t)
 	defer closer()
 
@@ -84,10 +85,7 @@ func TestDerivedTableWithHaving(t *testing.T) {
 	mcmp.Exec("insert into user(id, name) values(1,'toto'), (2,'tata'), (3,'titi'), (4,'tete'), (5,'foo')")
 
 	mcmp.Exec("set sql_mode = ''")
-
-	// this is probably flaky? the id returned from the derived table could be any of the ids from user.
-	// works on my machine (TM)
-	mcmp.Exec("select  /*vt+ PLANNER=Gen4 */ * from (select id from user having count(*) >= 1) s")
+	mcmp.AssertMatchesAnyNoCompare("select  /*vt+ PLANNER=Gen4 */ * from (select id from user having count(*) >= 1) s", "[[INT64(1)]]", "[[INT64(4)]]")
 }
 
 func TestDerivedTableColumns(t *testing.T) {
diff --git a/go/vt/vtgate/planbuilder/plan_test.go b/go/vt/vtgate/planbuilder/plan_test.go
index 70f89f88ec..11a5bf6a6b 100644
--- a/go/vt/vtgate/planbuilder/plan_test.go
+++ b/go/vt/vtgate/planbuilder/plan_test.go
@@ -29,6 +29,7 @@ import (
 	"strings"
 	"testing"
 
+	"github.com/nsf/jsondiff"
 	"github.com/stretchr/testify/require"
 
 	vtgatepb "vitess.io/vitess/go/vt/proto/vtgate"
@@ -40,8 +41,6 @@ import (
 	"vitess.io/vitess/go/mysql/collations"
 	"vitess.io/vitess/go/vt/vtgate/semantics"
 
-	"github.com/google/go-cmp/cmp"
-
 	vtrpcpb "vitess.io/vitess/go/vt/proto/vtrpc"
 	"vitess.io/vitess/go/vt/vterrors"
 
@@ -716,19 +715,9 @@ type (
 	}
 )
 
-func compacted(in string) string {
-	if in != "" && in[0] != '{' {
-		return in
-	}
-	dst := bytes.NewBuffer(nil)
-	err := json.Compact(dst, []byte(in))
-	if err != nil {
-		panic(err)
-	}
-	return dst.String()
-}
-
 func testFile(t *testing.T, filename, tempDir string, vschema *vschemaWrapper, render bool) {
+	opts := jsondiff.DefaultConsoleOptions()
+
 	t.Run(filename, func(t *testing.T) {
 		var expected []planTest
 		var outFirstPlanner string
@@ -756,10 +745,9 @@ func testFile(t *testing.T, filename, tempDir string, vschema *vschemaWrapper, r
 				}
 				out := getPlanOrErrorOutput(err, plan)
 
-				lft := compacted(out)
-				rgt := compacted(string(tcase.V3Plan))
-				if lft != rgt {
-					t.Errorf("V3 - %s\nDiff:\n%s\n[%s] \n[%s]", filename, cmp.Diff(tcase.V3Plan, out), tcase.V3Plan, out)
+				compare, s := jsondiff.Compare(tcase.V3Plan, []byte(out), &opts)
+				if compare != jsondiff.FullMatch {
+					t.Errorf("V3 - %s\nDiff:\n%s\n[%s] \n[%s]", filename, s, tcase.V3Plan, out)
 				}
 
 				outFirstPlanner = out
@@ -781,11 +769,9 @@ func testFile(t *testing.T, filename, tempDir string, vschema *vschemaWrapper, r
 			//       with this last expectation, it is an error if the Gen4 planner
 			//       produces the same plan as the V3 planner does
 			t.Run(fmt.Sprintf("Gen4: %s", testName), func(t *testing.T) {
-				x := string(tcase.Gen4Plan)
-				s := compacted(out)
-				s2 := compacted(x)
-				if s != s2 {
-					t.Errorf("Gen4 - %s\nDiff:\n%s\n[%s] \n[%s]", filename, cmp.Diff(s, s2), tcase.Gen4Plan, out)
+				compare, s := jsondiff.Compare(tcase.Gen4Plan, []byte(out), &opts)
+				if compare != jsondiff.FullMatch {
+					t.Errorf("Gen4 - %s\nDiff:\n%s\n[%s] \n[%s]", filename, s, tcase.Gen4Plan, out)
 				}
 
 				if outFirstPlanner == out {
diff --git a/go/vt/vtgate/planbuilder/testdata/select_cases.json b/go/vt/vtgate/planbuilder/testdata/select_cases.json
index 5157e1acc9..2e0d48ce9c 100644
--- a/go/vt/vtgate/planbuilder/testdata/select_cases.json
+++ b/go/vt/vtgate/planbuilder/testdata/select_cases.json
@@ -7870,5 +7870,87 @@
         "main.dual"
       ]
     }
+  },
+  {
+    "comment": "(OR 1 = 0) doesn't cause unnecessary scatter",
+    "query": "select * from user where id = 1 or 1 = 0",
+    "v3-plan": {
+      "QueryType": "SELECT",
+      "Original": "select * from user where id = 1 or 1 = 0",
+      "Instructions": {
+        "OperatorType": "Route",
+        "Variant": "Scatter",
+        "Keyspace": {
+          "Name": "user",
+          "Sharded": true
+        },
+        "FieldQuery": "select * from `user` where 1 != 1",
+        "Query": "select * from `user` where id = 1 or 1 = 0",
+        "Table": "`user`"
+      }
+    },
+    "gen4-plan": {
+      "QueryType": "SELECT",
+      "Original": "select * from user where id = 1 or 1 = 0",
+      "Instructions": {
+        "OperatorType": "Route",
+        "Variant": "EqualUnique",
+        "Keyspace": {
+          "Name": "user",
+          "Sharded": true
+        },
+        "FieldQuery": "select * from `user` where 1 != 1",
+        "Query": "select * from `user` where id = 1",
+        "Table": "`user`",
+        "Values": [
+          "INT64(1)"
+        ],
+        "Vindex": "user_index"
+      },
+      "TablesUsed": [
+        "user.user"
+      ]
+    }
+  },
+  {
+    "comment": "(OR 2 < 1) doesn't cause unnecessary scatter",
+    "query": "select * from user where id = 1 or 2 < 1",
+    "v3-plan": {
+      "QueryType": "SELECT",
+      "Original": "select * from user where id = 1 or 2 < 1",
+      "Instructions": {
+        "OperatorType": "Route",
+        "Variant": "Scatter",
+        "Keyspace": {
+          "Name": "user",
+          "Sharded": true
+        },
+        "FieldQuery": "select * from `user` where 1 != 1",
+        "Query": "select * from `user` where id = 1 or 2 < 1",
+        "Table": "`user`"
+      }
+    },
+    "gen4-plan": {
+      "QueryType": "SELECT",
+      "Original": "select * from user where id = 1 or 2 < 1",
+      "Instructions": {
+        "OperatorType": "Route",
+        "Variant": "EqualUnique",
+        "Keyspace": {
+          "Name": "user",
+          "Sharded": true
+        },
+        "FieldQuery": "select * from `user` where 1 != 1",
+        "Query": "select * from `user` where id = 1",
+        "Table": "`user`",
+        "Values": [
+          "INT64(1)"
+        ],
+        "Vindex": "user_index"
+      },
+      "TablesUsed": [
+        "user.user"
+      ]
+    }
   }
-]
\ No newline at end of file
+]
diff --git a/test/config.json b/test/config.json
index 7b0b8245f1..cb2dbf14c7 100644
--- a/test/config.json
+++ b/test/config.json
@@ -520,6 +520,15 @@
 			"RetryMax": 2,
 			"Tags": []
 		},
+		"vtgate_queries_derived": {
+			"File": "unused.go",
+			"Args": ["vitess.io/vitess/go/test/endtoend/vtgate/queries/derived"],
+			"Command": [],
+			"Manual": false,
+			"Shard": "vtgate_queries",
+			"RetryMax": 1,
+			"Tags": []
+		},
 		"vtgate_queries_aggregation": {
 			"File": "unused.go",
 			"Args": ["vitess.io/vitess/go/test/endtoend/vtgate/queries/aggregation"],
@@ -691,15 +700,6 @@
 			"RetryMax": 1,
 			"Tags": []
 		},
-		"vtgate_mysql80_derived": {
-			"File": "unused.go",
-			"Args": ["vitess.io/vitess/go/test/endtoend/vtgate/mysql80/derived"],
-			"Command": [],
-			"Manual": false,
-			"Shard": "mysql80",
-			"RetryMax": 1,
-			"Tags": []
-		},
 		"vtgate_sequence": {
 			"File": "unused.go",
 			"Args": ["vitess.io/vitess/go/test/endtoend/vtgate/sequence"],