search

yogthos / migratus

MIGRATE ALL THE THINGS!
642 stars 93 forks source link

Migration fails if a comment is present on line with semicolon #208

Open Rovanion opened 2 years ago

Rovanion commented 2 years ago

A migration like

select 2; -- Thing

will fail with the error

2022-01-13T18:58:54.278Z Skrutten ERROR [migratus.migration.sql:326] - failed to execute command:

select 2; -- Thing

2022-01-13T18:58:54.278Z Skrutten ERROR [migratus.migration.sql:326] - Too many update results were returned.
2022-01-13T18:58:54.278Z Skrutten ERROR [migratus.database:326] - Migration odlingshistorik failed because Batch entry 1 <unknown> was aborted: Too many update results were returned.  Call getNextException to see other errors in the batch. backing out
2022-01-13T18:58:54.280Z Skrutten INFO [migratus.core:326] - Ending migrations
Execution error (PSQLException) at org.postgresql.jdbc.BatchResultHandler/handleCommandStatus (BatchResultHandler.java:102).
Too many update results were returned.

while a migration like

select 1, -- Thing
2;

will work just fine. My guess would be that some sort of line splitting on semicolons is at hand, but that is just a wild guess.

yogthos commented 2 years ago

There is a special syntax for splitting multiple statements. It could be that ; in comments is tripping up the parser.

Rovanion commented 2 years ago

Probably related to that. Though note that there is no semicolon in the comments that cause the issue.

yogthos commented 2 years ago

Yeah, will have to take a closer look at this. If you have time to take a look in the meantime, a PR would be very welcome. :)

lispwright commented 12 months ago

This bug is easy to figure out and correct, and easy to avoid. Not wanting to get into parsing code (major reason I use LISPs!) would you like for now a PR to the README.md? If so where should that go? There's no bugs section, which might logically go just before Setup. This sort of single statement file contents thing is only covered in the Quick Start but it probably shouldn't be polluted with this.

I could also add on or more obvious things I noted like use :datasource instead of :connection when you've got the former.

Otherwise everything worked right the first time, thanks a lot!

yogthos commented 12 months ago

Maybe could add a gotchas section at the end for now, if you'd be up do a PR for that would be much appreciated.