mattes / migrate

Database migrations. CLI and Golang library.
Other
2.29k stars 326 forks source link

(Spanner) Add support for multiple statements in one migration step #281

Closed ashoda closed 7 years ago

ashoda commented 7 years ago

Proposed Change:

Support migration steps with multiple statements.

Current Behavior:

Migration steps with multiple statements currently fail with an error like Syntax error on line x, column x: Expecting \'EOF\' but found an unknown character (\";\")

Example of a migration that currently fails:

CREATE TABLE table_name (
    id STRING(255) NOT NULL,
) PRIMARY KEY(id);

CREATE INDEX table_name_id_idx ON table_name (id);
ashoda commented 7 years ago

@mattes any thoughts on adding multistatement support?

nicolaiskogheim commented 7 years ago

I would like migrations to not be restricted to a single statement. I'm new to this tool, and I'm surprised that that's the case. The reason is that I think some statements only makes sense together, as a unit, like in the example in the OP.

mattes commented 7 years ago

I agree on all of this. Unsure what the best way forward is. Maybe there needs to be some meta language after all.

ashoda commented 7 years ago

@mattes what do you think about what's implemented in this PR? The spanner client supports multiple statements for the DDL call. The statements just need to be passed in as a list of statement objects to the UpdateDatabaseDdl call, and that's what the proposed change accomplishes.

Are you thinking this behavior should be more explicit with the use of some meta language?

mattes commented 7 years ago

Are you thinking this behavior should be more explicit with the use of some meta language?

I more and more think that this is the way forward.

Let me merge this PR for now to make Spanner usable.