-
Notifications
You must be signed in to change notification settings - Fork 49
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[v2]change unsupported DDL behavior #137
[v2]change unsupported DDL behavior #137
Conversation
v2/loader/schema_parser_source.go
Outdated
@@ -58,13 +59,16 @@ func NewSchemaParserSource(fpath string) (SchemaSource, error) { | |||
v := tables[tableName] | |||
v.createIndexes = append(v.createIndexes, val) | |||
tables[tableName] = v | |||
case *spansql.CreateChangeStream: | |||
log.Printf("skipped. CreateChangeStream isn't supported yet. got ' %s'", ddlstmt.SQL()) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please remove this line. (If we do this, it's better to implement warning and it should output to stderr)
v2/loader/schema_parser_source.go
Outdated
default: | ||
return nil, fmt.Errorf("stmt should be CreateTable, CreateIndex or AlterTableAddForeignKey, but got '%s'", ddlstmt.SQL()) | ||
return nil, fmt.Errorf("stmt should be CreateTable, CreateIndex, CreateChangeStream or AlterTableAddForeignKey, but got '%s'", ddlstmt.SQL()) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's difficult to manage all expected key words here. Please change like this.
fmt.Errorf("unknown statement is specified: %s", ddlstmt.SQL())
v2/loader/schema_parser_source.go
Outdated
case *spansql.AlterTable: | ||
if isAlterTableAddFK(val) { | ||
continue | ||
} | ||
return nil, fmt.Errorf("stmt should be CreateTable, CreateIndex or AlterTableAddForeignKey, but got '%s'", ddlstmt.SQL()) | ||
return nil, fmt.Errorf("stmt should be CreateTable, CreateIndex, CreateChangeStream or AlterTableAddForeignKey, but got '%s'", ddlstmt.SQL()) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fmt.Errorf("unknown statement in AlterTable is specified: %s", ddlstmt.SQL())
Thanks for your review. |
We would like to skip when schema file includes
CREATE CHANGE STREAM
statements in v2.Instead of #116 ,
I change code to skip when schema file includes statement which can parse and unsupported in yo v2 as v1 do.