migratus icon indicating copy to clipboard operation
migratus copied to clipboard

Migration fails if a comment is present on line with semicolon

Open Rovanion opened this issue 3 years ago • 5 comments

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.

Rovanion avatar Jan 14 '22 12:01 Rovanion

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

yogthos avatar Jan 14 '22 15:01 yogthos

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

Rovanion avatar Jan 14 '22 23:01 Rovanion

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. :)

yogthos avatar Jan 15 '22 00:01 yogthos

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!

lispwright avatar Sep 12 '23 15:09 lispwright

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.

yogthos avatar Sep 12 '23 18:09 yogthos