Add use_transaction parameter for postgres_query() and postgres_execute()

[daniellietz]

Fixes #298

postgres_execute(db, query, use_transaction=TRUE);
postgres_query(db, query, use_transaction=TRUE);

use_transaction is an optional named parameter and set to true by default. If it is not mentioned, no behavior changes.
Not allowed to be turned off in read-only mode or inside an ongoing DuckDB transaction.

When turned off explicitly, it allows to execute queries that are not encapsulated by a transaction.
This allows to run operations such as VACUUM on your attached postgres database and gives you more control as to whether you want to use transactions in your queries or not.

Sample output:

D LOAD '~/projects/duckdb/duckdb-postgres/build/extension/postgres_scanner/postgres_scanner.duckdb_extension';
D SET pg_debug_show_queries=TRUE;
D ATTACH 'postgres://postgres@localhost:5432/postgres' AS pg (TYPE POSTGRES);
SELECT version(), (SELECT COUNT(*) FROM pg_settings WHERE name LIKE 'rds%')

D CALL postgres_execute('pg', 'VACUUM');
BEGIN TRANSACTION ISOLATION LEVEL REPEATABLE READ;
VACUUM

ROLLBACK

Invalid Error:
Failed to execute query "BEGIN TRANSACTION ISOLATION LEVEL REPEATABLE READ;
VACUUM": ERROR:  VACUUM cannot run inside a transaction block

D CALL postgres_execute('pg', 'VACUUM', use_transaction=FALSE);
VACUUM

┌─────────┐
│ Success │
│ boolean │
├─────────┤
│ 0 rows  │
└─────────┘
D CALL postgres_query('pg', 'SELECT 1');
BEGIN TRANSACTION ISOLATION LEVEL REPEATABLE READ

COPY (SELECT "?column?" FROM (SELECT 1) AS __unnamed_subquery ) TO STDOUT (FORMAT "binary");

COMMIT

┌──────────┐
│ ?column? │
│  int32   │
├──────────┤
│    1     │
└──────────┘
D CALL postgres_query('pg', 'SELECT 1', use_transaction=FALSE);
COPY (SELECT "?column?" FROM (SELECT 1) AS __unnamed_subquery ) TO STDOUT (FORMAT "binary");

┌──────────┐
│ ?column? │
│  int32   │
├──────────┤
│    1     │
└──────────┘

[daniellietz]

I am not sure why the pipeline is failing. The failures seem independent of the changes made in this PR. I have tried to bump DuckDB to v1.2.1 in the most recent commit.

[nojvek]

Really looking forward for this change to land. Currently post-hook using postgres_execute break due to transaction read level enforcement.

[daniellietz]

Hi @Mytherin , could you run the pipeline and take a look at this PR?
Many people were commenting under #298 that they were really interested in this feature.

[daniellietz]

Hi @Mytherin,
for both this and the limit pushdown PR the only failing test is the same one that also failed in #312 , which is on DuckDB side and independent from the changes made

[Mytherin]

I've fixed the CI in #320 - could you merge with main so this can re-run?

[daniellietz]

I've fixed the CI in #320 - could you merge with main so this can re-run?

done both here and in #313

[nojvek]

I acknowledge this may be a slight feature creep, but would it be possible to set a transaction isolation level as well during postgres_query and postgres_query.

REPEATABLE READ is a nice default, but in some instances you need a stronger guarantee like use_transaction='SERIALIZED'. Sometimes no guarantee is needed like use_transaction=False.

We are in need of having stronger guarantees for reads.

[Mytherin]

Thanks!

[Mytherin]

I acknowledge this may be a slight feature creep, but would it be possible to set a transaction isolation level as well during postgres_query and postgres_query.

REPEATABLE READ is a nice default, but in some instances you need a stronger guarantee like use_transaction='SERIALIZED'. Sometimes no guarantee is needed like use_transaction=False.

We are in need of having stronger guarantees for reads.

Could you open a separate issue for this? We could implement this - but I think it should be implemented as a configuration option.