Cherry pick additional planner fixes (vitessio#1662)

* [vtgate planner] Routing & Merging refactor (vitessio#12197) * start of extracting routing logic out from Route into an interface Signed-off-by: Andres Taylor <[email protected]> * wip - milestone - first plan-test green Signed-off-by: Andres Taylor <[email protected]> * wip - single table routes work fine Signed-off-by: Andres Taylor <[email protected]> * wip - sequence tables passing Signed-off-by: Andres Taylor <[email protected]> * wip - all tests in from_cases.json passing Signed-off-by: Andres Taylor <[email protected]> * wip - more work on merge logic Signed-off-by: Andres Taylor <[email protected]> * wip - need to switch the routes as well Signed-off-by: Andres Taylor <[email protected]> * subquery merging Signed-off-by: Harshit Gangal <[email protected]> * dual routing Signed-off-by: Harshit Gangal <[email protected]> * wip - make sure to keep inner/outer in the right place Signed-off-by: Andres Taylor <[email protected]> * make sure to not forget already seen predicates when merging sharded routes Signed-off-by: Andres Taylor <[email protected]> * add merged subqueries to the merged field Signed-off-by: Andres Taylor <[email protected]> * dual subquery and none routing change Signed-off-by: Harshit Gangal <[email protected]> * handle merging subqueries on IN comparisons Signed-off-by: Andres Taylor <[email protected]> * recalculate routing after merging subquery Signed-off-by: Andres Taylor <[email protected]> * rename field to make it easier to grokk Signed-off-by: Andres Taylor <[email protected]> * better merging logic when merging non-sharded tables Signed-off-by: Andres Taylor <[email protected]> * when merging subqueries, we must also copy predicates Signed-off-by: Andres Taylor <[email protected]> * copy keyspace when producing NoneRouting Signed-off-by: Andres Taylor <[email protected]> * implement Routing methods in TargetedRouting and update OpCode outside of shardedrouting Signed-off-by: Florent Poinsard <[email protected]> * Fix targeted routing update params and rest of the dml cases Signed-off-by: Florent Poinsard <[email protected]> * Enhanced UpdateRoutingLogic function to detect constant null and None routes Signed-off-by: Florent Poinsard <[email protected]> * clean up info schema route mergeing Signed-off-by: Andres Taylor <[email protected]> * handle reference tables with alternates Signed-off-by: Andres Taylor <[email protected]> * update remaining plan_tests Signed-off-by: Andres Taylor <[email protected]> * unify unsharded and reference routing in the same logic Signed-off-by: Andres Taylor <[email protected]> * clean up merging.go Signed-off-by: Andres Taylor <[email protected]> * build the alternate routes with the correct keyspace Signed-off-by: Andres Taylor <[email protected]> * minor fixes after self-review Signed-off-by: Andres Taylor <[email protected]> * when missing current keyspace, any valid keyspace can be used Signed-off-by: Andres Taylor <[email protected]> * final small changes. I promise Signed-off-by: Andres Taylor <[email protected]> * review feedback Signed-off-by: Andres Taylor <[email protected]> * tidy up method after review feedback Signed-off-by: Andres Taylor <[email protected]> * more cleanup - fix goland warnings in new files Signed-off-by: Andres Taylor <[email protected]> * add keyspace information to sequence routing Signed-off-by: Andres Taylor <[email protected]> * more cleanups from PR review Signed-off-by: Andres Taylor <[email protected]> * more cleanup Signed-off-by: Andres Taylor <[email protected]> --------- Signed-off-by: Andres Taylor <[email protected]> Signed-off-by: Harshit Gangal <[email protected]> Signed-off-by: Florent Poinsard <[email protected]> Co-authored-by: Harshit Gangal <[email protected]> Co-authored-by: Florent Poinsard <[email protected]> Signed-off-by: Dirkjan Bussink <[email protected]> * fix dual table handling (vitessio#12204) * fix planner: be careful when merging subqueries We were not checking dependencies of the outer subquery correctly, before merging with a route. This lead to invalid plans being produced. Signed-off-by: Andres Taylor <[email protected]> * fix planner: use tables on lhs when merging subqs When calculating if everything we need is available before merging routes, we now take into account anything coming from the LHS of any joins we are under. Signed-off-by: Andres Taylor <[email protected]> * test: add test merging subquery with join Signed-off-by: Andres Taylor <[email protected]> * use LHS from the top of the current node when planning RHSs Signed-off-by: Florent Poinsard <[email protected]> * added comments Signed-off-by: Andres Taylor <[email protected]> --------- Signed-off-by: Andres Taylor <[email protected]> Signed-off-by: Florent Poinsard <[email protected]> Co-authored-by: Florent Poinsard <[email protected]> Signed-off-by: Dirkjan Bussink <[email protected]> * Update tests to match main Signed-off-by: Dirkjan Bussink <[email protected]> * fix dual table handling (vitessio#12292) * fix dual table handling Signed-off-by: Andres Taylor <[email protected]> * update test expectations Signed-off-by: Andres Taylor <[email protected]> --------- Signed-off-by: Andres Taylor <[email protected]> Signed-off-by: Dirkjan Bussink <[email protected]> * Cleanup duplicate tests Signed-off-by: Dirkjan Bussink <[email protected]> * Fix endtoend tests because of cherry-pick Signed-off-by: Dirkjan Bussink <[email protected]> --------- Signed-off-by: Andres Taylor <[email protected]> Signed-off-by: Harshit Gangal <[email protected]> Signed-off-by: Florent Poinsard <[email protected]> Signed-off-by: Dirkjan Bussink <[email protected]> Co-authored-by: Andres Taylor <[email protected]> Co-authored-by: Harshit Gangal <[email protected]> Co-authored-by: Florent Poinsard <[email protected]>
planetscale · Mar 16, 2023 · 4300c38 · 4300c38
1 parent dfa82d4
commit 4300c38
Show file tree

Hide file tree

Showing 46 changed files with 2,587 additions and 5,129 deletions.
diff --git a/go/test/dbg/dbg.go b/go/test/dbg/dbg.go
@@ -23,6 +23,7 @@ import (
 	"go/format"
 	"go/parser"
 	"go/token"
+	"io"
 	"os"
 	"path"
 	"runtime"
@@ -163,6 +164,17 @@ func V[Val any](v Val) Val {
 
 // P prints all the arguments passed to the function in verbose debug form
 func P(vals ...any) {
+	dump(vals, os.Stdout)
+}
+
+// S returns a string with all the arguments passed to the function in verbose debug form
+func S(vals ...any) string {
+	buf := &strings.Builder{}
+	dump(vals, buf)
+	return buf.String()
+}
+
+func dump(vals []any, writer io.Writer) {
 	var p *params
 	if _, f, lineno, ok := runtime.Caller(1); ok {
 		p = defaultCache.resolve(f, lineno)
@@ -176,5 +188,7 @@ func P(vals ...any) {
 		w := text.NewIndentWriter(&buf, nil, bytes.Repeat([]byte{' '}, indent))
 		fmt.Fprintf(w, "%# v\n", pretty.Formatter(v))
 	}
-	_, _ = buf.WriteTo(os.Stdout)
+
+	_, _ = buf.WriteTo(writer)
+
 }
diff --git a/go/test/endtoend/vtgate/schematracker/restarttablet/schema_restart_test.go b/go/test/endtoend/vtgate/schematracker/restarttablet/schema_restart_test.go
@@ -122,7 +122,7 @@ func TestVSchemaTrackerInit(t *testing.T) {
 
 	qr := utils.Exec(t, conn, "SHOW VSCHEMA TABLES")
 	got := fmt.Sprintf("%v", qr.Rows)
-	want := `[[VARCHAR("dual")] [VARCHAR("main")] [VARCHAR("test_table")] [VARCHAR("vt_user")]]`
+	want := `[[VARCHAR("main")] [VARCHAR("test_table")] [VARCHAR("vt_user")]]`
 	assert.Equal(t, want, got)
 }
 

diff --git a/go/test/endtoend/vtgate/schematracker/sharded/st_sharded_test.go b/go/test/endtoend/vtgate/schematracker/sharded/st_sharded_test.go
@@ -160,7 +160,7 @@ func TestInitAndUpdate(t *testing.T) {
 	require.NoError(t, err)
 	defer conn.Close()
 
-	vtgateVersion, err := cluster.GetMajorVersion("vtgate")
+	vtgateVersion, err := 17, nil // cluster.GetMajorVersion("vtgate") -- backported to private, can be cleaned up to match upstream once rebased on 17
 	require.NoError(t, err)
 
 	expected := `[[VARCHAR("dual")] [VARCHAR("t2")] [VARCHAR("t2_id4_idx")] [VARCHAR("t8")]]`
@@ -223,7 +223,7 @@ func TestDMLOnNewTable(t *testing.T) {
 	// create a new table which is not part of the VSchema
 	utils.Exec(t, conn, `create table new_table_tracked(id bigint, name varchar(100), primary key(id)) Engine=InnoDB`)
 
-	vtgateVersion, err := cluster.GetMajorVersion("vtgate")
+	vtgateVersion, err := 17, nil // cluster.GetMajorVersion("vtgate") -- backported to private, can be cleaned up to match upstream once rebased on 17
 	require.NoError(t, err)
 	expected := `[[VARCHAR("dual")] [VARCHAR("new_table_tracked")] [VARCHAR("t2")] [VARCHAR("t2_id4_idx")] [VARCHAR("t8")]]`
 	if vtgateVersion >= 17 {

diff --git a/go/test/endtoend/vtgate/schematracker/unsharded/st_unsharded_test.go b/go/test/endtoend/vtgate/schematracker/unsharded/st_unsharded_test.go
@@ -95,7 +95,7 @@ func TestNewUnshardedTable(t *testing.T) {
 	require.NoError(t, err)
 	defer conn.Close()
 
-	vtgateVersion, err := cluster.GetMajorVersion("vtgate")
+	vtgateVersion, err := 17, nil // cluster.GetMajorVersion("vtgate") -- backported to private, can be cleaned up to match upstream once rebased on 17
 	require.NoError(t, err)
 	expected := `[[VARCHAR("dual")] [VARCHAR("main")]]`
 	if vtgateVersion >= 17 {

diff --git a/go/test/endtoend/vtgate/vschema/vschema_test.go b/go/test/endtoend/vtgate/vschema/vschema_test.go
@@ -111,7 +111,7 @@ func TestVSchema(t *testing.T) {
 
 	utils.AssertMatches(t, conn, "delete from vt_user", `[]`)
 
-	vtgateVersion, err := cluster.GetMajorVersion("vtgate")
+	vtgateVersion, err := 17, nil // cluster.GetMajorVersion("vtgate") -- backported to private, can be cleaned up to match upstream once rebased on 17
 	require.NoError(t, err)
 
 	// Test empty vschema

diff --git a/go/vt/sqlparser/ast.go b/go/vt/sqlparser/ast.go
@@ -2470,11 +2470,11 @@ type (
 	// This is a struct that the parser will never produce - it's written and read by the gen4 planner
 	// CAUTION: you should only change argName and hasValuesArg through the setter methods
 	ExtractedSubquery struct {
-		Original     Expr // original expression that was replaced by this ExtractedSubquery
-		OpCode       int  // this should really be engine.PulloutOpCode, but we cannot depend on engine :(
-		Subquery     *Subquery
-		OtherSide    Expr // represents the side of the comparison, this field will be nil if Original is not a comparison
-		NeedsRewrite bool // tells whether we need to rewrite this subquery to Original or not
+		Original  Expr // original expression that was replaced by this ExtractedSubquery
+		OpCode    int  // this should really be engine.PulloutOpCode, but we cannot depend on engine :(
+		Subquery  *Subquery
+		OtherSide Expr // represents the side of the comparison, this field will be nil if Original is not a comparison
+		Merged    bool // tells whether we need to rewrite this subquery to Original or not
 
 		hasValuesArg string
 		argName      string

diff --git a/go/vt/sqlparser/ast_equals.go b/go/vt/sqlparser/ast_equals.go