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

Signed-off-by: Harshit Gangal <harshit@planetscale.com>

Author: harshit-gangal

go/vt/vtgate/vschema_manager.go

@@ -22,6 +22,7 @@ import (
 
 	"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"
@@ -206,6 +207,10 @@ func (vm *VSchemaManager) updateFromSchema(vschema *vindexes.VSchema) {
 		// in the tables.
 		for tblName, tblInfo := range m {
 			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)

go/vt/vtgate/vschema_manager_test.go

@@ -372,6 +372,100 @@ func TestRebuildVSchema(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.ParseTable(parentTableName)
+	return &sqlparser.ForeignKeyDefinition{
+		Source: sqlparser.MakeColumns(childCols...),
+		ReferenceDefinition: &sqlparser.ReferenceDefinition{
+			ReferencedTable:   sqlparser.NewTableNameWithQualifier(pTbl, pKs),
+			ReferencedColumns: sqlparser.MakeColumns(parentCols...),
+			OnUpdate:          onUpdate,
+			OnDelete:          onDelete,
+		},
+	}
+}
+
 func makeTestVSchema(ks string, sharded bool, tbls map[string]*vindexes.Table) *vindexes.VSchema {
 	keyspaceSchema := &vindexes.KeyspaceSchema{
 		Keyspace: &vindexes.Keyspace{

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,
 		},
 	}
 

go/vt/vttablet/tabletserver/query_executor.go

@@ -1172,7 +1172,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})
 	})