Fix panic inside schema tracker

The schema tracker was not using strict DDL parsing which means that on an unparsable table definition, it would panic later on. The intent here always was from looking at the code to skip any table that can't be parsed properly, so let's do that here instead. Signed-off-by: Dirkjan Bussink <[email protected]>
vitessio · Jan 30, 2025 · bc0a441 · bc0a441
1 parent 92fe301
commit bc0a441
Show file tree

Hide file tree

Showing 4 changed files with 31 additions and 3 deletions.
diff --git a/go/vt/vtgate/schema/tracker.go b/go/vt/vtgate/schema/tracker.go
@@ -353,7 +353,7 @@ func (t *Tracker) updatedTableSchema(th *discovery.TabletHealth) bool {
 
 func (t *Tracker) updateTables(keyspace string, res map[string]string) {
 	for tableName, tableDef := range res {
-		stmt, err := t.parser.Parse(tableDef)
+		stmt, err := t.parser.ParseStrictDDL(tableDef)
 		if err != nil {
 			log.Warningf("error parsing table definition for %s: %v", tableName, err)
 			continue
@@ -535,7 +535,7 @@ func (vm *viewMap) set(ks, tbl, sql string) {
 		m = make(map[tableNameStr]sqlparser.TableStatement)
 		vm.m[ks] = m
 	}
-	stmt, err := vm.parser.Parse(sql)
+	stmt, err := vm.parser.ParseStrictDDL(sql)
 	if err != nil {
 		log.Warningf("ignoring view '%s', parsing error in view definition: '%s'", tbl, sql)
 		return

diff --git a/go/vt/vtgate/schema/tracker_test.go b/go/vt/vtgate/schema/tracker_test.go
@@ -202,6 +202,9 @@ func TestTableTracking(t *testing.T) {
 		tables(
 			tbl("t4", "create table t4(name varchar(50) primary key)"),
 		),
+		tables(
+			tbl("t5", "create table t5(name varchar(50) primary key with broken syntax)"),
+		),
 	}
 
 	testcases := []testCases{{
@@ -234,6 +237,15 @@ func TestTableTracking(t *testing.T) {
 			"t3": {{Name: sqlparser.NewIdentifierCI("id"), Type: querypb.Type_DATETIME, CollationName: "binary", Size: 0, Nullable: true}},
 			"t4": {{Name: sqlparser.NewIdentifierCI("name"), Type: querypb.Type_VARCHAR, Size: 50, Nullable: true}},
 		},
+	}, {
+		testName: "new broken table",
+		updTbl:   []string{"t5"},
+		expTbl: map[string][]vindexes.Column{
+			"t1": {{Name: sqlparser.NewIdentifierCI("id"), Type: querypb.Type_INT64, CollationName: "binary", Nullable: true}, {Name: sqlparser.NewIdentifierCI("name"), Type: querypb.Type_VARCHAR, Size: 50, Nullable: true}, {Name: sqlparser.NewIdentifierCI("email"), Type: querypb.Type_VARCHAR, Size: 50, Nullable: false, Default: &sqlparser.Literal{Val: "[email protected]"}}},
+			"T1": {{Name: sqlparser.NewIdentifierCI("id"), Type: querypb.Type_VARCHAR, Size: 50, Nullable: true}, {Name: sqlparser.NewIdentifierCI("name"), Type: querypb.Type_VARCHAR, Size: 50, Nullable: true}},
+			"t3": {{Name: sqlparser.NewIdentifierCI("id"), Type: querypb.Type_DATETIME, CollationName: "binary", Size: 0, Nullable: true}},
+			"t4": {{Name: sqlparser.NewIdentifierCI("name"), Type: querypb.Type_VARCHAR, Size: 50, Nullable: true}},
+		},
 	}}
 
 	testTracker(t, false, schemaResponse, testcases)
@@ -253,6 +265,7 @@ func TestViewsTracking(t *testing.T) {
 			tbl("t3", "create view t3 as select 1 from tbl3"),
 		),
 		tables(tbl("t4", "create view t4 as select 1 from tbl4")),
+		tables(tbl("t4", "create view t5 as select 1 from tbl4 with broken syntax")),
 	}
 
 	testcases := []testCases{{
@@ -281,6 +294,14 @@ func TestViewsTracking(t *testing.T) {
 			"V1": "select 1, 2 from tbl2",
 			"t3": "select 1 from tbl3",
 			"t4": "select 1 from tbl4"},
+	}, {
+		testName: "new broken t5",
+		updView:  []string{"t5"},
+		expView: map[string]string{
+			"t1": "select 1 from tbl1",
+			"V1": "select 1, 2 from tbl2",
+			"t3": "select 1 from tbl3",
+			"t4": "select 1 from tbl4"},
 	}}
 
 	testTracker(t, false, schemaDefResult, testcases)

diff --git a/go/vt/wrangler/vdiff.go b/go/vt/wrangler/vdiff.go
@@ -542,7 +542,7 @@ func findPKs(env *vtenv.Environment, table *tabletmanagerdatapb.TableDefinition,
 // column in the table definition leveraging MySQL's collation inheritance
 // rules.
 func getColumnCollations(venv *vtenv.Environment, table *tabletmanagerdatapb.TableDefinition) (map[string]collations.ID, map[string]*evalengine.EnumSetValues, error) {
-	createstmt, err := venv.Parser().Parse(table.Schema)
+	createstmt, err := venv.Parser().ParseStrictDDL(table.Schema)
 	if err != nil {
 		return nil, nil, err
 	}

diff --git a/go/vt/wrangler/vdiff_test.go b/go/vt/wrangler/vdiff_test.go
@@ -1311,6 +1311,13 @@ func TestGetColumnCollations(t *testing.T) {
 				"size": {"'small'", "'medium'", "'large'"},
 			},
 		},
+		{
+			name: "invalid schema",
+			table: &tabletmanagerdatapb.TableDefinition{
+				Schema: "create table t1 (c1 varchar(10), size set('small', 'medium', 'large'), primary key(c1) with syntax error)",
+			},
+			wantErr: true,
+		},
 	}
 	env := vtenv.NewTestEnv()
 	for _, tt := range tests {