Skip to content

Commit

Permalink
Fix schemadiff semantics handling
Browse files Browse the repository at this point in the history
The problem here is that we store entries in `.Tables` with their
unescaped names, which is entirely safe to do here.

However, we would use `sqlparser.String` to retrieve them. Now that
normally works for table names, unless a table name is a SQL keyword. In
that case, it turns into the escaped version with backticks and the
whole table can't be found for semantic analysis.

This can happen for example if you name a table `Order`.

Signed-off-by: Dirkjan Bussink <d.bussink@gmail.com>
  • Loading branch information
dbussink committed Jun 6, 2024
1 parent 6def783 commit 0998590
Show file tree
Hide file tree
Showing 3 changed files with 16 additions and 5 deletions.
10 changes: 6 additions & 4 deletions go/vt/schemadiff/schema_diff_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -415,13 +415,15 @@ func TestSchemaDiff(t *testing.T) {
instantCapability: InstantDDLCapabilityIrrelevant,
},
{
name: "add view with over",
name: "add view with over and keyword table",
toQueries: append(
createQueries,
"create view v2 as SELECT *, ROW_NUMBER() OVER(PARTITION BY info) AS row_num1, ROW_NUMBER() OVER(PARTITION BY info ORDER BY id) AS row_num2 FROM t1;\n",
"create table `order` (id int primary key, info int not null);",
"create view v2 as SELECT *, ROW_NUMBER() OVER(PARTITION BY info) AS row_num1, ROW_NUMBER() OVER(PARTITION BY info ORDER BY id) AS row_num2 FROM `order`;",
),
expectDiffs: 1,
entityOrder: []string{"v2"},
expectDiffs: 2,
expectDeps: 1,
entityOrder: []string{"order", "v2"},
instantCapability: InstantDDLCapabilityIrrelevant,
},
{
Expand Down
9 changes: 9 additions & 0 deletions go/vt/schemadiff/schema_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -125,6 +125,15 @@ func TestNewSchemaFromQueriesUnresolved(t *testing.T) {
assert.Equal(t, "CREATE VIEW `v7` AS SELECT * FROM `v8`, `t2`", v.Create().CanonicalStatementString())
}

func TestNewSchemaFromQueriesWithSQLKeyword(t *testing.T) {
queries := []string{
"create table `order` (id int primary key, info int not null)",
"create view v2 as SELECT *, ROW_NUMBER() OVER(PARTITION BY info) AS row_num1, ROW_NUMBER() OVER(PARTITION BY info ORDER BY id) AS row_num2 FROM `order`;",
}
_, err := NewSchemaFromQueries(NewTestEnv(), queries)
assert.NoError(t, err)
}

func TestNewSchemaFromQueriesUnresolvedAlias(t *testing.T) {
// v8 does not exist
queries := append(schemaTestCreateQueries,
Expand Down
2 changes: 1 addition & 1 deletion go/vt/schemadiff/semantics.go
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ func newDeclarativeSchemaInformation(env *Environment) *declarativeSchemaInforma

// FindTableOrVindex implements the SchemaInformation interface
func (si *declarativeSchemaInformation) FindTableOrVindex(tablename sqlparser.TableName) (*vindexes.Table, vindexes.Vindex, string, topodatapb.TabletType, key.Destination, error) {
table := si.Tables[sqlparser.String(tablename)]
table := si.Tables[tablename.Name.String()]
return table, nil, "", 0, nil, nil
}

Expand Down

0 comments on commit 0998590

Please sign in to comment.