dbmate icon indicating copy to clipboard operation
dbmate copied to clipboard

Getting pq: syntax error at or near "transaction" when running migration with transaction:false

Open brookback opened this issue 1 year ago • 6 comments

Description

Having

--migrate:up transaction:false

in a dbmate migration file produces a syntax error.

  • Version: dbmate version 2.4.0
  • Database: postgres v13
  • Operating System: macOS 12.6.3

Steps To Reproduce

  1. Do a postgres migration with CREATE INDEX CONCURRENTLY ... in a dbmate migrations file:
-- migrate:up transaction:false

drop index concurrently my_index;

create index concurrently my_index on my_table (
    some_column
);
  1. Run with dbmate --no-schema-dump up.

Actual Behavior

Applying: 20230621121155_my_migration.sql
Error: pq: syntax error at or near "transaction"

It seems this is some bug/limitation with the pq lib, but I just wanted to give a heads up.

brookback avatar Jun 22 '23 07:06 brookback

Taking a look at this, I've created similar migration:

-- migrate:up transaction:false
drop index concurrently idx;

create index concurrently idx on test (id);

I get a slightly different error which actually states it can't work within a transaction:

% dbmate up
Applying: 20230831142614_error.sql
Error: pq: DROP INDEX CONCURRENTLY cannot run inside a transaction block

Details:

  • Version: 2.6.0
  • Database: PostgreSQL 15.3
  • System: MacOS 13.4.1

Not got my test enviornment setup back up yet, I'll investigate more soon.

docapotamus avatar Aug 31 '23 14:08 docapotamus

I was able to reproduce this issue with the latest dbmate 2.7.0-main, against PostgreSQL 10.21:

root@c2a4e68d4451:/src# cat testdata/db/migrations/455_test_transaction_concurrently.sql 
-- migrate:up transaction:false

drop index concurrently my_index;

create index concurrently my_index on my_table (
    some_column
);

-- migrate:down

root@c2a4e68d4451:/src# dist/dbmate -u $POSTGRES_TEST_URL -d testdata/db/migrations up
Applying: 455_test_transaction_concurrently.sql
Error: pq: DROP INDEX CONCURRENTLY cannot be executed from a function or multi-command string

Searching for that particular error turns up lib/pq issue #820, and our own #126 that references it, where @amacneil wrote:

It sounds like this is a limitation of the lib/pq driver, and I'm not aware of any other golang drivers for postgresql. Therefore, I will close this with the workaround of putting each concurrent index creation in a separate migration file.

And this issue came up again in #182.

It looks like this is ultimately an issue with how lib/pq and Postgres handle transactions and support for multi-command queries, and isn't something that can be fixed from dbmate's end.

The workaround is to create separate migration files for each CREATE INDEX CONCURRENTLY command, which isn't great but it's simple and it works.

Perhaps the migrations option section of the README could document this, including the various error string responses above so that people searching may be able to find the information?

This is certainly a problem that has occurred repeatedly over the years, so I think there should be value in mentioning it clearly in the documentation.

dossy avatar Nov 15 '23 23:11 dossy

Duplicate of #126.

dossy avatar Nov 16 '23 03:11 dossy

does moving to pgx can solve this ?

Or using some kind of (not perfect ) regex ( ;[end of line]) to separate statements and run them one by one?

I think it can support a lot of use cases

RoiGlinik avatar May 22 '24 14:05 RoiGlinik

does moving to pgx can solve this ?

Or using some kind of (not perfect ) regex ( ;[end of line]) to separate statements and run them one by one?

I think it can support a lot of use cases

Moving to pgx is unlikely to solve this.

The author of pgx, @jackc, created their own SQL migration tool named tern. And, they encountered this very same issue, because the problem is not one that can be safely resolved at the driver level, in pgx.

Regular expressions alone cannot be used to create a fully correct SQL parser that could safely split any arbitrary SQL statement, although strictly limiting the splitting to ;\n might be a little safer, but still not 100% safe:

insert into t (data) values (
  'this is a;
long string'
);

You can see what @jackc did in tern, implementing their own minimal SQL parser in order to split statements, in this commit, b23e02f6. If you examine the tests they wrote, you can begin to see the complexity of possible cases that even a "simple" implementation must handle correctly.

dossy avatar May 23 '24 14:05 dossy