fix: ignore internal tables in schema tracking (vitessio#15141)

Signed-off-by: Harshit Gangal <harshit@planetscale.com>
planetscale · Feb 6, 2024 · 7a4f8a0 · 7a4f8a0
1 parent e182ca6
commit 7a4f8a0
Show file tree

Hide file tree

Showing 4 changed files with 160 additions and 51 deletions.
diff --git a/go/vt/vtgate/vschema_manager.go b/go/vt/vtgate/vschema_manager.go
@@ -23,6 +23,7 @@ import (
 	"vitess.io/vitess/go/vt/graph"
 	"vitess.io/vitess/go/vt/log"
 	topodatapb "vitess.io/vitess/go/vt/proto/topodata"
+	"vitess.io/vitess/go/vt/schema"
 	"vitess.io/vitess/go/vt/sqlparser"
 	"vitess.io/vitess/go/vt/srvtopo"
 	"vitess.io/vitess/go/vt/topo"
@@ -217,6 +218,10 @@ func (vm *VSchemaManager) updateFromSchema(vschema *vindexes.VSchema) {
 				continue
 			}
 			for _, fkDef := range tblInfo.ForeignKeys {
+				// Ignore internal tables as part of foreign key references.
+				if schema.IsInternalOperationTableName(fkDef.ReferenceDefinition.ReferencedTable.Name.String()) {
+					continue
+				}
 				parentTbl, err := vschema.FindRoutedTable(ksName, fkDef.ReferenceDefinition.ReferencedTable.Name.String(), topodatapb.TabletType_PRIMARY)
 				if err != nil {
 					log.Errorf("error finding parent table %s: %v", fkDef.ReferenceDefinition.ReferencedTable.Name.String(), err)

diff --git a/go/vt/vtgate/vschema_manager_test.go b/go/vt/vtgate/vschema_manager_test.go
@@ -438,56 +438,6 @@ func TestRebuildVSchema(t *testing.T) {
 	}
 }
 
-func makeTestVSchema(ks string, sharded bool, tbls map[string]*vindexes.Table) *vindexes.VSchema {
-	keyspaceSchema := &vindexes.KeyspaceSchema{
-		Keyspace: &vindexes.Keyspace{
-			Name:    ks,
-			Sharded: sharded,
-		},
-		// Default foreign key mode
-		ForeignKeyMode: vschemapb.Keyspace_unmanaged,
-		Tables:         tbls,
-		Vindexes:       map[string]vindexes.Vindex{},
-	}
-	vs := makeTestEmptyVSchema()
-	vs.Keyspaces[ks] = keyspaceSchema
-	vs.ResetCreated()
-	return vs
-}
-
-func makeTestEmptyVSchema() *vindexes.VSchema {
-	return &vindexes.VSchema{
-		RoutingRules: map[string]*vindexes.RoutingRule{},
-		Keyspaces:    map[string]*vindexes.KeyspaceSchema{},
-	}
-}
-
-func makeTestSrvVSchema(ks string, sharded bool, tbls map[string]*vschemapb.Table) *vschemapb.SrvVSchema {
-	keyspaceSchema := &vschemapb.Keyspace{
-		Sharded: sharded,
-		Tables:  tbls,
-		// Default foreign key mode
-		ForeignKeyMode: vschemapb.Keyspace_unmanaged,
-	}
-	return &vschemapb.SrvVSchema{
-		Keyspaces: map[string]*vschemapb.Keyspace{ks: keyspaceSchema},
-	}
-}
-
-type fakeSchema struct {
-	t map[string]*vindexes.TableInfo
-}
-
-func (f *fakeSchema) Tables(string) map[string]*vindexes.TableInfo {
-	return f.t
-}
-
-func (f *fakeSchema) Views(string) map[string]sqlparser.SelectStatement {
-	return nil
-}
-
-var _ SchemaInfo = (*fakeSchema)(nil)
-
 func TestMarkErrorIfCyclesInFk(t *testing.T) {
 	ksName := "ks"
 	keyspace := &vindexes.Keyspace{
@@ -573,6 +523,86 @@ func TestMarkErrorIfCyclesInFk(t *testing.T) {
 	}
 }
 
+// TestVSchemaUpdateWithFKReferenceToInternalTables tests that any internal table as part of fk reference is ignored.
+func TestVSchemaUpdateWithFKReferenceToInternalTables(t *testing.T) {
+	ks := &vindexes.Keyspace{Name: "ks"}
+	cols1 := []vindexes.Column{{
+		Name: sqlparser.NewIdentifierCI("id"),
+		Type: querypb.Type_INT64,
+	}}
+	sqlparserCols1 := sqlparser.MakeColumns("id")
+
+	vindexTable_t1 := &vindexes.Table{
+		Name:                    sqlparser.NewIdentifierCS("t1"),
+		Keyspace:                ks,
+		Columns:                 cols1,
+		ColumnListAuthoritative: true,
+	}
+	vindexTable_t2 := &vindexes.Table{
+		Name:                    sqlparser.NewIdentifierCS("t2"),
+		Keyspace:                ks,
+		Columns:                 cols1,
+		ColumnListAuthoritative: true,
+	}
+
+	vindexTable_t1.ChildForeignKeys = append(vindexTable_t1.ChildForeignKeys, vindexes.ChildFKInfo{
+		Table:         vindexTable_t2,
+		ChildColumns:  sqlparserCols1,
+		ParentColumns: sqlparserCols1,
+		OnDelete:      sqlparser.SetNull,
+		OnUpdate:      sqlparser.Cascade,
+	})
+	vindexTable_t2.ParentForeignKeys = append(vindexTable_t2.ParentForeignKeys, vindexes.ParentFKInfo{
+		Table:         vindexTable_t1,
+		ChildColumns:  sqlparserCols1,
+		ParentColumns: sqlparserCols1,
+	})
+
+	vm := &VSchemaManager{}
+	var vs *vindexes.VSchema
+	vm.subscriber = func(vschema *vindexes.VSchema, _ *VSchemaStats) {
+		vs = vschema
+		vs.ResetCreated()
+	}
+	vm.schema = &fakeSchema{t: map[string]*vindexes.TableInfo{
+		"t1": {Columns: cols1},
+		"t2": {
+			Columns: cols1,
+			ForeignKeys: []*sqlparser.ForeignKeyDefinition{
+				createFkDefinition([]string{"id"}, "t1", []string{"id"}, sqlparser.Cascade, sqlparser.SetNull),
+				createFkDefinition([]string{"id"}, "_vt_HOLD_6ace8bcef73211ea87e9f875a4d24e90_20200915120410", []string{"id"}, sqlparser.Cascade, sqlparser.SetNull),
+			},
+		},
+	}}
+	vm.VSchemaUpdate(&vschemapb.SrvVSchema{
+		Keyspaces: map[string]*vschemapb.Keyspace{
+			"ks": {
+				ForeignKeyMode: vschemapb.Keyspace_managed,
+				Tables: map[string]*vschemapb.Table{
+					"t1": {Columns: []*vschemapb.Column{{Name: "id", Type: querypb.Type_INT64}}},
+					"t2": {Columns: []*vschemapb.Column{{Name: "id", Type: querypb.Type_INT64}}},
+				},
+			},
+		},
+	}, nil)
+
+	utils.MustMatchFn(".globalTables", ".uniqueVindexes")(t, &vindexes.VSchema{
+		RoutingRules: map[string]*vindexes.RoutingRule{},
+		Keyspaces: map[string]*vindexes.KeyspaceSchema{
+			"ks": {
+				Keyspace:       ks,
+				ForeignKeyMode: vschemapb.Keyspace_managed,
+				Vindexes:       map[string]vindexes.Vindex{},
+				Tables: map[string]*vindexes.Table{
+					"t1": vindexTable_t1,
+					"t2": vindexTable_t2,
+				},
+			},
+		},
+	}, vs)
+	utils.MustMatch(t, vs, vm.currentVschema, "currentVschema should have same reference as Vschema")
+}
+
 // createFkDefinition is a helper function to create a Foreign key definition struct from the columns used in it provided as list of strings.
 func createFkDefinition(childCols []string, parentTableName string, parentCols []string, onUpdate, onDelete sqlparser.ReferenceAction) *sqlparser.ForeignKeyDefinition {
 	pKs, pTbl, _ := sqlparser.NewTestParser().ParseTable(parentTableName)
@@ -586,3 +616,53 @@ func createFkDefinition(childCols []string, parentTableName string, parentCols [
 		},
 	}
 }
+
+func makeTestVSchema(ks string, sharded bool, tbls map[string]*vindexes.Table) *vindexes.VSchema {
+	keyspaceSchema := &vindexes.KeyspaceSchema{
+		Keyspace: &vindexes.Keyspace{
+			Name:    ks,
+			Sharded: sharded,
+		},
+		// Default foreign key mode
+		ForeignKeyMode: vschemapb.Keyspace_unmanaged,
+		Tables:         tbls,
+		Vindexes:       map[string]vindexes.Vindex{},
+	}
+	vs := makeTestEmptyVSchema()
+	vs.Keyspaces[ks] = keyspaceSchema
+	vs.ResetCreated()
+	return vs
+}
+
+func makeTestEmptyVSchema() *vindexes.VSchema {
+	return &vindexes.VSchema{
+		RoutingRules: map[string]*vindexes.RoutingRule{},
+		Keyspaces:    map[string]*vindexes.KeyspaceSchema{},
+	}
+}
+
+func makeTestSrvVSchema(ks string, sharded bool, tbls map[string]*vschemapb.Table) *vschemapb.SrvVSchema {
+	keyspaceSchema := &vschemapb.Keyspace{
+		Sharded: sharded,
+		Tables:  tbls,
+		// Default foreign key mode
+		ForeignKeyMode: vschemapb.Keyspace_unmanaged,
+	}
+	return &vschemapb.SrvVSchema{
+		Keyspaces: map[string]*vschemapb.Keyspace{ks: keyspaceSchema},
+	}
+}
+
+type fakeSchema struct {
+	t map[string]*vindexes.TableInfo
+}
+
+func (f *fakeSchema) Tables(string) map[string]*vindexes.TableInfo {
+	return f.t
+}
+
+func (f *fakeSchema) Views(string) map[string]sqlparser.SelectStatement {
+	return nil
+}
+
+var _ SchemaInfo = (*fakeSchema)(nil)
diff --git a/go/vt/vttablet/endtoend/rpc_test.go b/go/vt/vttablet/endtoend/rpc_test.go
@@ -169,6 +169,25 @@ func TestGetSchemaRPC(t *testing.T) {
 			},
 			getSchemaQueryType: querypb.SchemaTableType_ALL,
 			getSchemaTables:    []string{"vitess_temp1", "vitess_temp3", "unknown_table", "vitess_view3", "vitess_view1", "unknown_view"},
+		}, {
+			name: "Create some internal tables",
+			queries: []string{
+				"create table if not exists _vt_HOLD_6ace8bcef73211ea87e9f875a4d24e90_20200915120410(id bigint primary key);",
+				"create table vitess_temp1 (eid int);",
+				"create view vitess_view1 as select eid from vitess_a",
+			},
+			deferQueries: []string{
+				"drop table _vt_HOLD_6ace8bcef73211ea87e9f875a4d24e90_20200915120410",
+				"drop table vitess_temp1",
+				"drop view vitess_view1",
+			},
+			mapToExpect: map[string]string{
+				"vitess_view1": "CREATE ALGORITHM=UNDEFINED DEFINER=`vt_dba`@`localhost` SQL SECURITY DEFINER VIEW `vitess_view1` AS select `vitess_a`.`eid` AS `eid` from `vitess_a`",
+				"vitess_temp1": "CREATE TABLE `vitess_temp1` (\n  `eid` int DEFAULT NULL\n) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_0900_ai_ci",
+				// These shouldn't be part of the result, so we verify it is empty.
+				"_vt_HOLD_6ace8bcef73211ea87e9f875a4d24e90_20200915120410": "",
+			},
+			getSchemaQueryType: querypb.SchemaTableType_ALL,
 		},
 	}
 

diff --git a/go/vt/vttablet/tabletserver/query_executor.go b/go/vt/vttablet/tabletserver/query_executor.go
@@ -1182,7 +1182,12 @@ func (qre *QueryExecutor) executeGetSchemaQuery(query string, callback func(sche
 	return qre.execStreamSQL(conn, false /* isTransaction */, query, func(result *sqltypes.Result) error {
 		schemaDef := make(map[string]string)
 		for _, row := range result.Rows {
-			schemaDef[row[0].ToString()] = row[1].ToString()
+			tableName := row[0].ToString()
+			// Schema RPC should ignore the internal table in the response.
+			if schema.IsInternalOperationTableName(tableName) {
+				continue
+			}
+			schemaDef[tableName] = row[1].ToString()
 		}
 		return callback(&querypb.GetSchemaResponse{TableDefinition: schemaDef})
 	})