From 50e2fc0c1814204328ec30321f989c623985741a Mon Sep 17 00:00:00 2001 From: Angela Xie Date: Wed, 28 May 2025 14:15:02 -0700 Subject: [PATCH 1/6] escape single quotes in column comment --- enginetest/queries/create_table_queries.go | 24 +++++++++++----------- sql/parser.go | 9 +++++--- 2 files changed, 18 insertions(+), 15 deletions(-) diff --git a/enginetest/queries/create_table_queries.go b/enginetest/queries/create_table_queries.go index b96b4a8556..7937148cb9 100644 --- a/enginetest/queries/create_table_queries.go +++ b/enginetest/queries/create_table_queries.go @@ -40,6 +40,18 @@ var CreateTableQueries = []WriteQueryTest{ SelectQuery: "SHOW CREATE TABLE tableWithComment", ExpectedSelect: []sql.Row{{"tableWithComment", "CREATE TABLE `tableWithComment` (\n `pk` int\n) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_0900_bin COMMENT='~!@ #$ %^ &* ()'"}}, }, + { + WriteQuery: `create table tableWithComment (pk int) COMMENT "'"`, + ExpectedWriteResult: []sql.Row{{types.NewOkResult(0)}}, + SelectQuery: "SHOW CREATE TABLE tableWithComment", + ExpectedSelect: []sql.Row{{"tableWithComment", "CREATE TABLE `tableWithComment` (\n `pk` int\n) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_0900_bin COMMENT=''''"}}, + }, + { + WriteQuery: `create table tableWithColumnComment (pk int COMMENT "'")`, + ExpectedWriteResult: []sql.Row{{types.NewOkResult(0)}}, + SelectQuery: "SHOW CREATE TABLE tableWithColumnComment", + ExpectedSelect: []sql.Row{{"tableWithColumnComment", "CREATE TABLE `tableWithColumnComment` (\n `pk` int COMMENT ''''\n) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_0900_bin"}}, + }, { WriteQuery: `create table floattypedefs (a float(10), b float(10, 2), c double(10, 2))`, ExpectedWriteResult: []sql.Row{{types.NewOkResult(0)}}, @@ -1118,18 +1130,6 @@ var CreateTableAutoIncrementTests = []ScriptTest{ } var BrokenCreateTableQueries = []WriteQueryTest{ - { - // TODO: We don't support table comments that contain single quotes due to how table options are parsed. - // Vitess parses them, but turns any double quotes into single quotes and puts all table options back - // into a string that GMS has to reparse. This means we can't tell if the single quote is the end of - // the quoted string, or if it was a single quote inside of double quotes and needs to be escaped. - // To fix this, Vitess should return the parsed table options as structured data, instead of as a - // single string. - WriteQuery: `create table tableWithComment (pk int) COMMENT "'"`, - ExpectedWriteResult: []sql.Row{{types.NewOkResult(0)}}, - SelectQuery: "SHOW CREATE TABLE tableWithComment", - ExpectedSelect: []sql.Row{{"tableWithComment", "CREATE TABLE `tableWithComment` (\n `pk` int\n) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_0900_bin COMMENT=''''"}}, - }, { WriteQuery: `create table t1 (b blob, primary key(b(1)))`, ExpectedWriteResult: []sql.Row{{types.NewOkResult(0)}}, diff --git a/sql/parser.go b/sql/parser.go index 48d894f349..14b0fd22cb 100644 --- a/sql/parser.go +++ b/sql/parser.go @@ -133,6 +133,10 @@ func RemoveSpaceAndDelimiter(query string, d rune) string { }) } +func EscapeSingleQuotesInComment(comment string) string { + return strings.ReplaceAll(comment, "'", "''") +} + type MySqlSchemaFormatter struct{} var _ SchemaFormatter = &MySqlSchemaFormatter{} @@ -141,8 +145,7 @@ var _ SchemaFormatter = &MySqlSchemaFormatter{} func (m *MySqlSchemaFormatter) GenerateCreateTableStatement(tblName string, colStmts []string, temp, autoInc, tblCharsetName, tblCollName, comment string) string { if comment != "" { // Escape any single quotes in the comment and add the COMMENT keyword - comment = strings.ReplaceAll(comment, "'", "''") - comment = fmt.Sprintf(" COMMENT='%s'", comment) + comment = fmt.Sprintf(" COMMENT='%s'", EscapeSingleQuotesInComment(comment)) } if autoInc != "" { @@ -201,7 +204,7 @@ func (m *MySqlSchemaFormatter) GenerateCreateTableColumnDefinition(col *Column, } if col.Comment != "" { - stmt = fmt.Sprintf("%s COMMENT '%s'", stmt, col.Comment) + stmt = fmt.Sprintf("%s COMMENT '%s'", stmt, EscapeSingleQuotesInComment(col.Comment)) } return stmt } From c2a1ecd1fbcef9709dbf2870c0ac53938c9b4e98 Mon Sep 17 00:00:00 2001 From: Angela Xie Date: Wed, 28 May 2025 16:27:26 -0700 Subject: [PATCH 2/6] asdf --- enginetest/memory_engine_test.go | 18 ++++++++---------- sql/parser.go | 19 +++++++++++++++---- 2 files changed, 23 insertions(+), 14 deletions(-) diff --git a/enginetest/memory_engine_test.go b/enginetest/memory_engine_test.go index cb5455eed8..2d46ee91a9 100644 --- a/enginetest/memory_engine_test.go +++ b/enginetest/memory_engine_test.go @@ -202,20 +202,18 @@ func TestSingleQueryPrepared(t *testing.T) { // Convenience test for debugging a single query. Unskip and set to the desired query. func TestSingleScript(t *testing.T) { - t.Skip() + // t.Skip() var scripts = []queries.ScriptTest{ { - Name: "AS OF propagates to nested CALLs", - SetUpScript: []string{}, + Name: "AS OF propagates to nested CALLs", + SetUpScript: []string{ + `create table t (i int comment "single quote \' | newline \n | return \r | backslash \\ | NUL \0 \x00");`, + //"create table t (pk int comment 'this, it''s the way to go')", + //`create table t (pk int comment "\'")`, + }, Assertions: []queries.ScriptTestAssertion{ { - Query: "create procedure create_proc() create table t (i int primary key, j int);", - Expected: []sql.Row{ - {types.NewOkResult(0)}, - }, - }, - { - Query: "call create_proc()", + Query: "show create table t", Expected: []sql.Row{ {types.NewOkResult(0)}, }, diff --git a/sql/parser.go b/sql/parser.go index 14b0fd22cb..05e0cdabab 100644 --- a/sql/parser.go +++ b/sql/parser.go @@ -133,8 +133,17 @@ func RemoveSpaceAndDelimiter(query string, d rune) string { }) } -func EscapeSingleQuotesInComment(comment string) string { - return strings.ReplaceAll(comment, "'", "''") +func EscapeSpecialCharactersInComment(comment string) string { + panic("here!") + commentString := comment + commentString = strings.ReplaceAll(commentString, "'", "''") + commentString = strings.ReplaceAll(commentString, "\\", "\\\\") + commentString = strings.ReplaceAll(commentString, "\"", "\\\"") + commentString = strings.ReplaceAll(commentString, "\n", "\\n") + commentString = strings.ReplaceAll(commentString, "\r", "\\r") + + commentString = strings.ReplaceAll(commentString, "\x00", "\\0") + return commentString } type MySqlSchemaFormatter struct{} @@ -143,9 +152,10 @@ var _ SchemaFormatter = &MySqlSchemaFormatter{} // GenerateCreateTableStatement implements the SchemaFormatter interface. func (m *MySqlSchemaFormatter) GenerateCreateTableStatement(tblName string, colStmts []string, temp, autoInc, tblCharsetName, tblCollName, comment string) string { + panic("here! 1") if comment != "" { // Escape any single quotes in the comment and add the COMMENT keyword - comment = fmt.Sprintf(" COMMENT='%s'", EscapeSingleQuotesInComment(comment)) + comment = fmt.Sprintf(" COMMENT='%s'", EscapeSpecialCharactersInComment(comment)) } if autoInc != "" { @@ -166,6 +176,7 @@ func (m *MySqlSchemaFormatter) GenerateCreateTableStatement(tblName string, colS // GenerateCreateTableColumnDefinition implements the SchemaFormatter interface. func (m *MySqlSchemaFormatter) GenerateCreateTableColumnDefinition(col *Column, colDefault, onUpdate string, tableCollation CollationID) string { + panic("here! 2") var colTypeString string if collationType, ok := col.Type.(TypeWithCollation); ok { colTypeString = collationType.StringWithTableCollation(tableCollation) @@ -204,7 +215,7 @@ func (m *MySqlSchemaFormatter) GenerateCreateTableColumnDefinition(col *Column, } if col.Comment != "" { - stmt = fmt.Sprintf("%s COMMENT '%s'", stmt, EscapeSingleQuotesInComment(col.Comment)) + stmt = fmt.Sprintf("%s COMMENT '%s'", stmt, EscapeSpecialCharactersInComment(col.Comment)) } return stmt } From 5b482eab6704895dc4b2413a1eed0da3084f1b24 Mon Sep 17 00:00:00 2001 From: Angela Xie Date: Wed, 28 May 2025 18:23:12 -0700 Subject: [PATCH 3/6] escape backslashed characters in comments --- enginetest/queries/create_table_queries.go | 12 ++++++++++++ sql/parser.go | 4 ---- 2 files changed, 12 insertions(+), 4 deletions(-) diff --git a/enginetest/queries/create_table_queries.go b/enginetest/queries/create_table_queries.go index 7937148cb9..aca8607b77 100644 --- a/enginetest/queries/create_table_queries.go +++ b/enginetest/queries/create_table_queries.go @@ -52,6 +52,18 @@ var CreateTableQueries = []WriteQueryTest{ SelectQuery: "SHOW CREATE TABLE tableWithColumnComment", ExpectedSelect: []sql.Row{{"tableWithColumnComment", "CREATE TABLE `tableWithColumnComment` (\n `pk` int COMMENT ''''\n) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_0900_bin"}}, }, + { + WriteQuery: `create table tableWithColumnComment (pk int COMMENT "\'")`, + ExpectedWriteResult: []sql.Row{{types.NewOkResult(0)}}, + SelectQuery: "SHOW CREATE TABLE tableWithColumnComment", + ExpectedSelect: []sql.Row{{"tableWithColumnComment", "CREATE TABLE `tableWithColumnComment` (\n `pk` int COMMENT ''''\n) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_0900_bin"}}, + }, + { + WriteQuery: `create table tableWithColumnComment (pk int COMMENT "newline \n | return \r | backslash \\ | NUL \0 \x00")`, + ExpectedWriteResult: []sql.Row{{types.NewOkResult(0)}}, + SelectQuery: "SHOW CREATE TABLE tableWithColumnComment", + ExpectedSelect: []sql.Row{{"tableWithColumnComment", "CREATE TABLE `tableWithColumnComment` (\n `pk` int COMMENT 'newline \\n | return \\r | backslash \\\\ | NUL \\0 x00'\n) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_0900_bin"}}, + }, { WriteQuery: `create table floattypedefs (a float(10), b float(10, 2), c double(10, 2))`, ExpectedWriteResult: []sql.Row{{types.NewOkResult(0)}}, diff --git a/sql/parser.go b/sql/parser.go index 05e0cdabab..ea4703f6fa 100644 --- a/sql/parser.go +++ b/sql/parser.go @@ -134,14 +134,12 @@ func RemoveSpaceAndDelimiter(query string, d rune) string { } func EscapeSpecialCharactersInComment(comment string) string { - panic("here!") commentString := comment commentString = strings.ReplaceAll(commentString, "'", "''") commentString = strings.ReplaceAll(commentString, "\\", "\\\\") commentString = strings.ReplaceAll(commentString, "\"", "\\\"") commentString = strings.ReplaceAll(commentString, "\n", "\\n") commentString = strings.ReplaceAll(commentString, "\r", "\\r") - commentString = strings.ReplaceAll(commentString, "\x00", "\\0") return commentString } @@ -152,7 +150,6 @@ var _ SchemaFormatter = &MySqlSchemaFormatter{} // GenerateCreateTableStatement implements the SchemaFormatter interface. func (m *MySqlSchemaFormatter) GenerateCreateTableStatement(tblName string, colStmts []string, temp, autoInc, tblCharsetName, tblCollName, comment string) string { - panic("here! 1") if comment != "" { // Escape any single quotes in the comment and add the COMMENT keyword comment = fmt.Sprintf(" COMMENT='%s'", EscapeSpecialCharactersInComment(comment)) @@ -176,7 +173,6 @@ func (m *MySqlSchemaFormatter) GenerateCreateTableStatement(tblName string, colS // GenerateCreateTableColumnDefinition implements the SchemaFormatter interface. func (m *MySqlSchemaFormatter) GenerateCreateTableColumnDefinition(col *Column, colDefault, onUpdate string, tableCollation CollationID) string { - panic("here! 2") var colTypeString string if collationType, ok := col.Type.(TypeWithCollation); ok { colTypeString = collationType.StringWithTableCollation(tableCollation) From 0706c5f110297f5eecb69f61c6b2d6e34310fc0b Mon Sep 17 00:00:00 2001 From: Angela Xie Date: Wed, 28 May 2025 18:35:25 -0700 Subject: [PATCH 4/6] remove memory_engine_test changes --- enginetest/memory_engine_test.go | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/enginetest/memory_engine_test.go b/enginetest/memory_engine_test.go index 2d46ee91a9..cb5455eed8 100644 --- a/enginetest/memory_engine_test.go +++ b/enginetest/memory_engine_test.go @@ -202,18 +202,20 @@ func TestSingleQueryPrepared(t *testing.T) { // Convenience test for debugging a single query. Unskip and set to the desired query. func TestSingleScript(t *testing.T) { - // t.Skip() + t.Skip() var scripts = []queries.ScriptTest{ { - Name: "AS OF propagates to nested CALLs", - SetUpScript: []string{ - `create table t (i int comment "single quote \' | newline \n | return \r | backslash \\ | NUL \0 \x00");`, - //"create table t (pk int comment 'this, it''s the way to go')", - //`create table t (pk int comment "\'")`, - }, + Name: "AS OF propagates to nested CALLs", + SetUpScript: []string{}, Assertions: []queries.ScriptTestAssertion{ { - Query: "show create table t", + Query: "create procedure create_proc() create table t (i int primary key, j int);", + Expected: []sql.Row{ + {types.NewOkResult(0)}, + }, + }, + { + Query: "call create_proc()", Expected: []sql.Row{ {types.NewOkResult(0)}, }, From 2dd4300d9fff92efd8c7f515dc4981481e8382ca Mon Sep 17 00:00:00 2001 From: Angela Xie Date: Thu, 29 May 2025 11:35:22 -0700 Subject: [PATCH 5/6] added more table comment tests --- enginetest/queries/create_table_queries.go | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/enginetest/queries/create_table_queries.go b/enginetest/queries/create_table_queries.go index aca8607b77..0a0ba057b0 100644 --- a/enginetest/queries/create_table_queries.go +++ b/enginetest/queries/create_table_queries.go @@ -46,6 +46,18 @@ var CreateTableQueries = []WriteQueryTest{ SelectQuery: "SHOW CREATE TABLE tableWithComment", ExpectedSelect: []sql.Row{{"tableWithComment", "CREATE TABLE `tableWithComment` (\n `pk` int\n) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_0900_bin COMMENT=''''"}}, }, + { + WriteQuery: `create table tableWithComment (pk int) COMMENT "\'"`, + ExpectedWriteResult: []sql.Row{{types.NewOkResult(0)}}, + SelectQuery: "SHOW CREATE TABLE tableWithComment", + ExpectedSelect: []sql.Row{{"tableWithComment", "CREATE TABLE `tableWithComment` (\n `pk` int\n) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_0900_bin COMMENT=''''"}}, + }, + { + WriteQuery: `create table tableWithComment (pk int) COMMENT "newline \n | return \r | backslash \\ | NUL \0 \x00"`, + ExpectedWriteResult: []sql.Row{{types.NewOkResult(0)}}, + SelectQuery: "SHOW CREATE TABLE tableWithComment", + ExpectedSelect: []sql.Row{{"tableWithComment", "CREATE TABLE `tableWithComment` (\n `pk` int\n) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_0900_bin COMMENT='newline \\n | return \\r | backslash \\\\ | NUL \\0 x00'"}}, + }, { WriteQuery: `create table tableWithColumnComment (pk int COMMENT "'")`, ExpectedWriteResult: []sql.Row{{types.NewOkResult(0)}}, From f722c6023cd1375b62695868c3cbd3898212205a Mon Sep 17 00:00:00 2001 From: Angela Xie Date: Thu, 29 May 2025 12:44:02 -0700 Subject: [PATCH 6/6] added test for altering column comments --- enginetest/queries/alter_table_queries.go | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/enginetest/queries/alter_table_queries.go b/enginetest/queries/alter_table_queries.go index a250f03fcb..a6103a8aff 100644 --- a/enginetest/queries/alter_table_queries.go +++ b/enginetest/queries/alter_table_queries.go @@ -1029,6 +1029,23 @@ var AlterTableScripts = []ScriptTest{ }, }, }, + { + Name: "alter table comments are escaped", + SetUpScript: []string{ + "create table t (i int);", + `alter table t modify column i int comment "newline \n | return \r | backslash \\ | NUL \0 \x00"`, + `alter table t add column j int comment "newline \n | return \r | backslash \\ | NUL \0 \x00"`, + }, + Assertions: []ScriptTestAssertion{ + { + Query: "show create table t", + Expected: []sql.Row{{ + "t", + "CREATE TABLE `t` (\n `i` int COMMENT 'newline \\n | return \\r | backslash \\\\ | NUL \\0 x00'," + + "\n `j` int COMMENT 'newline \\n | return \\r | backslash \\\\ | NUL \\0 x00'\n) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_0900_bin"}}, + }, + }, + }, } var RenameTableScripts = []ScriptTest{