diff --git a/contrib/postgres_fdw/connection.c b/contrib/postgres_fdw/connection.c index 192f8011160..06673017bcf 100644 --- a/contrib/postgres_fdw/connection.c +++ b/contrib/postgres_fdw/connection.c @@ -60,6 +60,7 @@ typedef struct ConnCacheEntry /* Remaining fields are invalid when conn is NULL: */ int xact_depth; /* 0 = no xact open, 1 = main xact open, 2 = * one level of subxact open, etc */ + bool xact_read_only; /* xact r/o state */ bool have_prep_stmt; /* have we prepared any stmts in this xact? */ bool have_error; /* have any subxacts aborted in this xact? */ bool changing_xact_state; /* xact state change in process */ @@ -86,6 +87,12 @@ static unsigned int prep_stmt_number = 0; /* tracks whether any work is needed in callback functions */ static bool xact_got_connection = false; +/* + * tracks the topmost read-only local transaction's nesting level determined + * by GetTopReadOnlyTransactionNestLevel() + */ +static int read_only_level = 0; + /* custom wait event values, retrieved from shared memory */ static uint32 pgfdw_we_cleanup_result = 0; static uint32 pgfdw_we_connect = 0; @@ -378,6 +385,7 @@ make_new_connection(ConnCacheEntry *entry, UserMapping *user) /* Reset all transient state fields, to be sure all are clean */ entry->xact_depth = 0; + entry->xact_read_only = false; entry->have_prep_stmt = false; entry->have_error = false; entry->changing_xact_state = false; @@ -871,29 +879,106 @@ do_sql_command_end(PGconn *conn, const char *sql, bool consume_input) * those scans. A disadvantage is that we can't provide sane emulation of * READ COMMITTED behavior --- it would be nice if we had some other way to * control which remote queries share a snapshot. + * + * Note also that we always start the remote transaction with the same + * read/write and deferrable properties as the local transaction, and start + * the remote subtransaction with the same read/write property as the local + * subtransaction. */ static void begin_remote_xact(ConnCacheEntry *entry) { int curlevel = GetCurrentTransactionNestLevel(); - /* Start main transaction if we haven't yet */ + /* + * If the current local (sub)transaction is read-only, set the topmost + * read-only local transaction's nesting level if we haven't yet. + * + * Note: once it's set, it's retained until the topmost read-only local + * transaction is committed/aborted (see pgfdw_xact_callback and + * pgfdw_subxact_callback). + */ + if (XactReadOnly) + { + if (read_only_level == 0) + read_only_level = GetTopReadOnlyTransactionNestLevel(); + Assert(read_only_level > 0); + } + else + Assert(read_only_level == 0); + + /* + * Start main transaction if we haven't yet; otherwise, change the current + * remote (sub)transaction's read/write mode if needed. + */ if (entry->xact_depth <= 0) { - const char *sql; + /* + * This is the case when we haven't yet started a main transaction. + */ + StringInfoData sql; + bool ro = (read_only_level == 1); elog(DEBUG3, "starting remote transaction on connection %p", entry->conn); + initStringInfo(&sql); + appendStringInfoString(&sql, "START TRANSACTION ISOLATION LEVEL "); if (IsolationIsSerializable()) - sql = "START TRANSACTION ISOLATION LEVEL SERIALIZABLE"; + appendStringInfoString(&sql, "SERIALIZABLE"); else - sql = "START TRANSACTION ISOLATION LEVEL REPEATABLE READ"; + appendStringInfoString(&sql, "REPEATABLE READ"); + if (ro) + appendStringInfoString(&sql, " READ ONLY"); + if (XactDeferrable) + appendStringInfoString(&sql, " DEFERRABLE"); entry->changing_xact_state = true; - do_sql_command(entry->conn, sql); + do_sql_command(entry->conn, sql.data); entry->xact_depth = 1; + if (ro) + { + Assert(!entry->xact_read_only); + entry->xact_read_only = true; + } entry->changing_xact_state = false; } + else if (!entry->xact_read_only) + { + /* + * The remote (sub)transaction has been opened in read-write mode. + */ + Assert(read_only_level == 0 || + entry->xact_depth <= read_only_level); + + /* + * If its nesting depth matches read_only_level, it means that the + * local read-write (sub)transaction that started it has changed to + * read-only after that; in which case change it to read-only as well. + * Otherwise, the local (sub)transaction is still read-write, so there + * is no need to do anything. + */ + if (entry->xact_depth == read_only_level) + { + entry->changing_xact_state = true; + do_sql_command(entry->conn, "SET transaction_read_only = on"); + entry->xact_read_only = true; + entry->changing_xact_state = false; + } + } + else + { + /* + * The remote (sub)transaction has been opened in read-only mode. + */ + Assert(read_only_level > 0 && + entry->xact_depth >= read_only_level); + + /* + * The local read-only (sub)transaction that started it is guaranteed + * to be still read-only (see check_transaction_read_only), so there + * is no need to do anything. + */ + } /* * If we're in a subtransaction, stack up savepoints to match our level. @@ -902,12 +987,21 @@ begin_remote_xact(ConnCacheEntry *entry) */ while (entry->xact_depth < curlevel) { - char sql[64]; + StringInfoData sql; + bool ro = (entry->xact_depth + 1 == read_only_level); - snprintf(sql, sizeof(sql), "SAVEPOINT s%d", entry->xact_depth + 1); + initStringInfo(&sql); + appendStringInfo(&sql, "SAVEPOINT s%d", entry->xact_depth + 1); + if (ro) + appendStringInfoString(&sql, "; SET transaction_read_only = on"); entry->changing_xact_state = true; - do_sql_command(entry->conn, sql); + do_sql_command(entry->conn, sql.data); entry->xact_depth++; + if (ro) + { + Assert(!entry->xact_read_only); + entry->xact_read_only = true; + } entry->changing_xact_state = false; } } @@ -1212,6 +1306,9 @@ pgfdw_xact_callback(XactEvent event, void *arg) /* Also reset cursor numbering for next transaction */ cursor_number = 0; + + /* Likewise for read_only_level */ + read_only_level = 0; } /* @@ -1310,6 +1407,10 @@ pgfdw_subxact_callback(SubXactEvent event, SubTransactionId mySubid, false); } } + + /* If in read_only_level, reset it */ + if (curlevel == read_only_level) + read_only_level = 0; } /* @@ -1412,6 +1513,9 @@ pgfdw_reset_xact_state(ConnCacheEntry *entry, bool toplevel) /* Reset state to show we're out of a transaction */ entry->xact_depth = 0; + /* Reset xact r/o state */ + entry->xact_read_only = false; + /* * If the connection isn't in a good idle state, it is marked as * invalid or keep_connections option of its server is disabled, then @@ -1432,6 +1536,10 @@ pgfdw_reset_xact_state(ConnCacheEntry *entry, bool toplevel) { /* Reset state to show we're out of a subtransaction */ entry->xact_depth--; + + /* If in read_only_level, reset xact r/o state */ + if (entry->xact_depth + 1 == read_only_level) + entry->xact_read_only = false; } } diff --git a/contrib/postgres_fdw/expected/postgres_fdw.out b/contrib/postgres_fdw/expected/postgres_fdw.out index ac34a1acacb..cd22553236f 100644 --- a/contrib/postgres_fdw/expected/postgres_fdw.out +++ b/contrib/postgres_fdw/expected/postgres_fdw.out @@ -12575,6 +12575,142 @@ SELECT count(*) FROM remote_application_name DROP FOREIGN TABLE remote_application_name; DROP VIEW my_application_name; -- =================================================================== +-- test read-only and/or deferrable transactions +-- =================================================================== +CREATE TABLE loct (f1 int, f2 text); +CREATE FUNCTION locf() RETURNS SETOF loct LANGUAGE SQL AS + 'UPDATE public.loct SET f2 = f2 || f2 RETURNING *'; +CREATE VIEW locv AS SELECT t.* FROM locf() t; +CREATE FOREIGN TABLE remt (f1 int, f2 text) + SERVER loopback OPTIONS (table_name 'locv'); +CREATE FOREIGN TABLE remt2 (f1 int, f2 text) + SERVER loopback2 OPTIONS (table_name 'locv'); +INSERT INTO loct VALUES (1, 'foo'), (2, 'bar'); +START TRANSACTION READ ONLY; +SAVEPOINT s; +SELECT * FROM remt; -- should fail +ERROR: cannot execute UPDATE in a read-only transaction +CONTEXT: SQL function "locf" statement 1 +remote SQL command: SELECT f1, f2 FROM public.locv +ROLLBACK TO s; +RELEASE SAVEPOINT s; +SELECT * FROM remt; -- should fail +ERROR: cannot execute UPDATE in a read-only transaction +CONTEXT: SQL function "locf" statement 1 +remote SQL command: SELECT f1, f2 FROM public.locv +ROLLBACK; +START TRANSACTION; +SAVEPOINT s; +SET transaction_read_only = on; +SELECT * FROM remt; -- should fail +ERROR: cannot execute UPDATE in a read-only transaction +CONTEXT: SQL function "locf" statement 1 +remote SQL command: SELECT f1, f2 FROM public.locv +ROLLBACK TO s; +RELEASE SAVEPOINT s; +SET transaction_read_only = on; +SELECT * FROM remt; -- should fail +ERROR: cannot execute UPDATE in a read-only transaction +CONTEXT: SQL function "locf" statement 1 +remote SQL command: SELECT f1, f2 FROM public.locv +ROLLBACK; +START TRANSACTION; +SAVEPOINT s; +SELECT * FROM remt; -- should work + f1 | f2 +----+-------- + 1 | foofoo + 2 | barbar +(2 rows) + +SET transaction_read_only = on; +SELECT * FROM remt; -- should fail +ERROR: cannot execute UPDATE in a read-only transaction +CONTEXT: SQL function "locf" statement 1 +remote SQL command: SELECT f1, f2 FROM public.locv +ROLLBACK TO s; +RELEASE SAVEPOINT s; +SELECT * FROM remt; -- should work + f1 | f2 +----+-------- + 1 | foofoo + 2 | barbar +(2 rows) + +SET transaction_read_only = on; +SELECT * FROM remt; -- should fail +ERROR: cannot execute UPDATE in a read-only transaction +CONTEXT: SQL function "locf" statement 1 +remote SQL command: SELECT f1, f2 FROM public.locv +ROLLBACK; +-- Exercise abort code paths in pgfdw_xact_callback/pgfdw_subxact_callback +-- in situations where multiple connections are involved +START TRANSACTION; +SAVEPOINT s; +SELECT * FROM remt; -- should work + f1 | f2 +----+-------- + 1 | foofoo + 2 | barbar +(2 rows) + +SET transaction_read_only = on; +SELECT * FROM remt2; -- should fail +ERROR: cannot execute UPDATE in a read-only transaction +CONTEXT: SQL function "locf" statement 1 +remote SQL command: SELECT f1, f2 FROM public.locv +ROLLBACK TO s; +RELEASE SAVEPOINT s; +SELECT * FROM remt; -- should work + f1 | f2 +----+-------- + 1 | foofoo + 2 | barbar +(2 rows) + +SET transaction_read_only = on; +SELECT * FROM remt2; -- should fail +ERROR: cannot execute UPDATE in a read-only transaction +CONTEXT: SQL function "locf" statement 1 +remote SQL command: SELECT f1, f2 FROM public.locv +ROLLBACK; +DROP FOREIGN TABLE remt; +CREATE FOREIGN TABLE remt (f1 int, f2 text) + SERVER loopback OPTIONS (table_name 'loct'); +START TRANSACTION ISOLATION LEVEL SERIALIZABLE READ ONLY; +SELECT * FROM remt; + f1 | f2 +----+----- + 1 | foo + 2 | bar +(2 rows) + +COMMIT; +START TRANSACTION ISOLATION LEVEL SERIALIZABLE DEFERRABLE; +SELECT * FROM remt; + f1 | f2 +----+----- + 1 | foo + 2 | bar +(2 rows) + +COMMIT; +START TRANSACTION ISOLATION LEVEL SERIALIZABLE READ ONLY DEFERRABLE; +SELECT * FROM remt; + f1 | f2 +----+----- + 1 | foo + 2 | bar +(2 rows) + +COMMIT; +-- Clean up +DROP FOREIGN TABLE remt; +DROP FOREIGN TABLE remt2; +DROP VIEW locv; +DROP FUNCTION locf(); +DROP TABLE loct; +-- =================================================================== -- test parallel commit and parallel abort -- =================================================================== ALTER SERVER loopback OPTIONS (ADD parallel_commit 'true'); diff --git a/contrib/postgres_fdw/sql/postgres_fdw.sql b/contrib/postgres_fdw/sql/postgres_fdw.sql index 0e218b29a29..59963e298b8 100644 --- a/contrib/postgres_fdw/sql/postgres_fdw.sql +++ b/contrib/postgres_fdw/sql/postgres_fdw.sql @@ -4328,6 +4328,86 @@ SELECT count(*) FROM remote_application_name DROP FOREIGN TABLE remote_application_name; DROP VIEW my_application_name; +-- =================================================================== +-- test read-only and/or deferrable transactions +-- =================================================================== +CREATE TABLE loct (f1 int, f2 text); +CREATE FUNCTION locf() RETURNS SETOF loct LANGUAGE SQL AS + 'UPDATE public.loct SET f2 = f2 || f2 RETURNING *'; +CREATE VIEW locv AS SELECT t.* FROM locf() t; +CREATE FOREIGN TABLE remt (f1 int, f2 text) + SERVER loopback OPTIONS (table_name 'locv'); +CREATE FOREIGN TABLE remt2 (f1 int, f2 text) + SERVER loopback2 OPTIONS (table_name 'locv'); +INSERT INTO loct VALUES (1, 'foo'), (2, 'bar'); + +START TRANSACTION READ ONLY; +SAVEPOINT s; +SELECT * FROM remt; -- should fail +ROLLBACK TO s; +RELEASE SAVEPOINT s; +SELECT * FROM remt; -- should fail +ROLLBACK; + +START TRANSACTION; +SAVEPOINT s; +SET transaction_read_only = on; +SELECT * FROM remt; -- should fail +ROLLBACK TO s; +RELEASE SAVEPOINT s; +SET transaction_read_only = on; +SELECT * FROM remt; -- should fail +ROLLBACK; + +START TRANSACTION; +SAVEPOINT s; +SELECT * FROM remt; -- should work +SET transaction_read_only = on; +SELECT * FROM remt; -- should fail +ROLLBACK TO s; +RELEASE SAVEPOINT s; +SELECT * FROM remt; -- should work +SET transaction_read_only = on; +SELECT * FROM remt; -- should fail +ROLLBACK; + +-- Exercise abort code paths in pgfdw_xact_callback/pgfdw_subxact_callback +-- in situations where multiple connections are involved +START TRANSACTION; +SAVEPOINT s; +SELECT * FROM remt; -- should work +SET transaction_read_only = on; +SELECT * FROM remt2; -- should fail +ROLLBACK TO s; +RELEASE SAVEPOINT s; +SELECT * FROM remt; -- should work +SET transaction_read_only = on; +SELECT * FROM remt2; -- should fail +ROLLBACK; + +DROP FOREIGN TABLE remt; +CREATE FOREIGN TABLE remt (f1 int, f2 text) + SERVER loopback OPTIONS (table_name 'loct'); + +START TRANSACTION ISOLATION LEVEL SERIALIZABLE READ ONLY; +SELECT * FROM remt; +COMMIT; + +START TRANSACTION ISOLATION LEVEL SERIALIZABLE DEFERRABLE; +SELECT * FROM remt; +COMMIT; + +START TRANSACTION ISOLATION LEVEL SERIALIZABLE READ ONLY DEFERRABLE; +SELECT * FROM remt; +COMMIT; + +-- Clean up +DROP FOREIGN TABLE remt; +DROP FOREIGN TABLE remt2; +DROP VIEW locv; +DROP FUNCTION locf(); +DROP TABLE loct; + -- =================================================================== -- test parallel commit and parallel abort -- =================================================================== diff --git a/doc/src/sgml/postgres-fdw.sgml b/doc/src/sgml/postgres-fdw.sgml index de69ddcdebc..9185c76f932 100644 --- a/doc/src/sgml/postgres-fdw.sgml +++ b/doc/src/sgml/postgres-fdw.sgml @@ -1103,6 +1103,23 @@ CREATE SUBSCRIPTION my_subscription SERVER subscription_server PUBLICATION testp PostgreSQL release might modify these rules. + + The remote transaction is opened in the same read/write mode as the local + transaction: if the local transaction is READ ONLY, + the remote transaction is opened in READ ONLY mode, + otherwise it is opened in READ WRITE mode. + (This rule is also applied to remote and local subtransactions.) + Note that this does not prevent login triggers executed on the remote + server from writing. + + + + The remote transaction is also opened in the same deferrable mode as the + local transaction: if the local transaction is DEFERRABLE, + the remote transaction is opened in DEFERRABLE mode, + otherwise it is opened in NOT DEFERRABLE mode. + + Note that it is currently not supported by postgres_fdw to prepare the remote transaction for diff --git a/src/backend/access/transam/xact.c b/src/backend/access/transam/xact.c index aafc53e0164..48bc90c9673 100644 --- a/src/backend/access/transam/xact.c +++ b/src/backend/access/transam/xact.c @@ -1046,6 +1046,34 @@ TransactionStartedDuringRecovery(void) return CurrentTransactionState->startedInRecovery; } +/* + * GetTopReadOnlyTransactionNestLevel + * + * Note: this will return zero when not inside any transaction or when neither + * a top-level transaction nor subtransactions are read-only, one when the + * top-level transaction is read-only, two when one level of subtransaction is + * read-only, etc. + * + * Note: subtransactions of the topmost read-only transaction are also + * read-only, because they inherit read-only mode from the transaction, and + * thus can't change to read-write mode (see check_transaction_read_only). + */ +int +GetTopReadOnlyTransactionNestLevel(void) +{ + TransactionState s = CurrentTransactionState; + + if (!XactReadOnly) + return 0; + while (s->nestingLevel > 1) + { + if (!s->prevXactReadOnly) + return s->nestingLevel; + s = s->parent; + } + return s->nestingLevel; +} + /* * EnterParallelMode */ diff --git a/src/include/access/xact.h b/src/include/access/xact.h index f0b4d795071..a8cbdf247c8 100644 --- a/src/include/access/xact.h +++ b/src/include/access/xact.h @@ -459,6 +459,7 @@ extern TimestampTz GetCurrentTransactionStopTimestamp(void); extern void SetCurrentStatementStartTimestamp(void); extern int GetCurrentTransactionNestLevel(void); extern bool TransactionIdIsCurrentTransactionId(TransactionId xid); +extern int GetTopReadOnlyTransactionNestLevel(void); extern void CommandCounterIncrement(void); extern void ForceSyncCommit(void); extern void StartTransactionCommand(void);