From bc0a44117cbc388da9962d545b360288c5120b40 Mon Sep 17 00:00:00 2001
From: Dirkjan Bussink <d.bussink@gmail.com>
Date: Thu, 30 Jan 2025 16:06:56 +0100
Subject: [PATCH] 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 <d.bussink@gmail.com>
---
 go/vt/vtgate/schema/tracker.go      |  4 ++--
 go/vt/vtgate/schema/tracker_test.go | 21 +++++++++++++++++++++
 go/vt/wrangler/vdiff.go             |  2 +-
 go/vt/wrangler/vdiff_test.go        |  7 +++++++
 4 files changed, 31 insertions(+), 3 deletions(-)

diff --git a/go/vt/vtgate/schema/tracker.go b/go/vt/vtgate/schema/tracker.go
index 8fa41712223..4dae11cded9 100644
--- 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
index 1ee15ba99cb..e5280c78ed8 100644
--- 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: "a@b.com"}}},
+			"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
index 2e9fc5f508d..9991b5bd633 100644
--- 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
index 3ac6edb373c..44c3a22a336 100644
--- 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 {