]> git.ipfire.org Git - thirdparty/postgresql.git/commitdiff
Revert "postgres_fdw: Inherit the local transaction's access/deferrable modes."
authorEtsuro Fujita <efujita@postgresql.org>
Sun, 8 Jun 2025 08:30:00 +0000 (17:30 +0900)
committerEtsuro Fujita <efujita@postgresql.org>
Sun, 8 Jun 2025 08:30:00 +0000 (17:30 +0900)
We concluded that commit e5a3c9d9b is a feature rather than a fix; since
it was added after feature freeze, revert it.

Reported-by: Fujii Masao <masao.fujii@oss.nttdata.com>
Reported-by: Michael Paquier <michael@paquier.xyz>
Reported-by: Robert Haas <robertmhaas@gmail.com>
Discussion: https://postgr.es/m/ed2296f1-1a6b-4932-b870-5bb18c2591ae%40oss.nttdata.com

contrib/postgres_fdw/connection.c
contrib/postgres_fdw/expected/postgres_fdw.out
contrib/postgres_fdw/sql/postgres_fdw.sql
doc/src/sgml/postgres-fdw.sgml
src/backend/access/transam/xact.c
src/include/access/xact.h

index caf1446269635b80189e8a31afcd033e47b4f3b0..304f3c20f835697d352dbfb91b1b11d54e6afc0a 100644 (file)
@@ -58,7 +58,6 @@ 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 */
@@ -85,12 +84,6 @@ static unsigned int prep_stmt_number = 0;
 /* tracks whether any work is needed in callback functions */
 static bool xact_got_connection = false;
 
-/*
- * tracks the nesting level of the topmost read-only transaction determined
- * by GetTopReadOnlyTransactionNestLevel()
- */
-static int     top_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;
@@ -379,7 +372,6 @@ 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;
@@ -851,81 +843,29 @@ 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();
 
-       /*
-        * Set the nesting level of the topmost read-only transaction if the
-        * current transaction is read-only and we haven't yet.  Once it's set,
-        * it's retained until that transaction is committed/aborted, and then
-        * reset (see pgfdw_xact_callback and pgfdw_subxact_callback).
-        */
-       if (XactReadOnly)
-       {
-               if (top_read_only_level == 0)
-                       top_read_only_level = GetTopReadOnlyTransactionNestLevel();
-               Assert(top_read_only_level > 0);
-       }
-       else
-               Assert(top_read_only_level == 0);
-
-       /*
-        * Start main transaction if we haven't yet; otherwise, change the
-        * already-started remote transaction/subtransaction to read-only if the
-        * local transaction/subtransaction have been done so after starting them
-        * and we haven't yet.
-        */
+       /* Start main transaction if we haven't yet */
        if (entry->xact_depth <= 0)
        {
-               StringInfoData sql;
-               bool            ro = (top_read_only_level == 1);
+               const char *sql;
 
                elog(DEBUG3, "starting remote transaction on connection %p",
                         entry->conn);
 
-               initStringInfo(&sql);
-               appendStringInfoString(&sql, "START TRANSACTION ISOLATION LEVEL ");
                if (IsolationIsSerializable())
-                       appendStringInfoString(&sql, "SERIALIZABLE");
+                       sql = "START TRANSACTION ISOLATION LEVEL SERIALIZABLE";
                else
-                       appendStringInfoString(&sql, "REPEATABLE READ");
-               if (ro)
-                       appendStringInfoString(&sql, " READ ONLY");
-               if (XactDeferrable)
-                       appendStringInfoString(&sql, " DEFERRABLE");
+                       sql = "START TRANSACTION ISOLATION LEVEL REPEATABLE READ";
                entry->changing_xact_state = true;
-               do_sql_command(entry->conn, sql.data);
+               do_sql_command(entry->conn, sql);
                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)
-       {
-               Assert(top_read_only_level == 0 ||
-                          entry->xact_depth <= top_read_only_level);
-               if (entry->xact_depth == top_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
-               Assert(top_read_only_level > 0 &&
-                          entry->xact_depth >= top_read_only_level);
 
        /*
         * If we're in a subtransaction, stack up savepoints to match our level.
@@ -934,21 +874,12 @@ begin_remote_xact(ConnCacheEntry *entry)
         */
        while (entry->xact_depth < curlevel)
        {
-               StringInfoData sql;
-               bool            ro = (entry->xact_depth + 1 == top_read_only_level);
+               char            sql[64];
 
-               initStringInfo(&sql);
-               appendStringInfo(&sql, "SAVEPOINT s%d", entry->xact_depth + 1);
-               if (ro)
-                       appendStringInfoString(&sql, "; SET transaction_read_only = on");
+               snprintf(sql, sizeof(sql), "SAVEPOINT s%d", entry->xact_depth + 1);
                entry->changing_xact_state = true;
-               do_sql_command(entry->conn, sql.data);
+               do_sql_command(entry->conn, sql);
                entry->xact_depth++;
-               if (ro)
-               {
-                       Assert(!entry->xact_read_only);
-                       entry->xact_read_only = true;
-               }
                entry->changing_xact_state = false;
        }
 }
@@ -1243,9 +1174,6 @@ pgfdw_xact_callback(XactEvent event, void *arg)
 
        /* Also reset cursor numbering for next transaction */
        cursor_number = 0;
-
-       /* Likewise for top_read_only_level */
-       top_read_only_level = 0;
 }
 
 /*
@@ -1344,10 +1272,6 @@ pgfdw_subxact_callback(SubXactEvent event, SubTransactionId mySubid,
                                                                           false);
                }
        }
-
-       /* If in the topmost read-only transaction, reset top_read_only_level */
-       if (curlevel == top_read_only_level)
-               top_read_only_level = 0;
 }
 
 /*
@@ -1450,9 +1374,6 @@ 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
@@ -1473,10 +1394,6 @@ pgfdw_reset_xact_state(ConnCacheEntry *entry, bool toplevel)
        {
                /* Reset state to show we're out of a subtransaction */
                entry->xact_depth--;
-
-               /* If in the topmost read-only transaction, reset xact r/o state */
-               if (entry->xact_depth + 1 == top_read_only_level)
-                       entry->xact_read_only = false;
        }
 }
 
index eb4716bed813277cfbc391831ee930a82854c2f8..2185b42bb4f79f976dc6d8ed4936dae3ce468c70 100644 (file)
@@ -12384,140 +12384,6 @@ 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;
-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');
index 20a535b99d82f6177dc45e21c3afdd3500f8401f..e534b40de3c76e133eefbbc0b898d885310a5c22 100644 (file)
@@ -4200,84 +4200,6 @@ 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;
-
-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
 -- ===================================================================
index c464716e3ce192cccf4e450e8b25b982d0459efc..781a01067f7d6d0c8ca15b7aba68f040646d655a 100644 (file)
@@ -1077,21 +1077,6 @@ postgres=# SELECT postgres_fdw_disconnect_all();
    <productname>PostgreSQL</productname> release might modify these rules.
   </para>
 
-  <para>
-   The remote transaction is opened in the same read/write mode as the local
-   transaction: if the local transaction is <literal>READ ONLY</literal>,
-   the remote transaction is opened in <literal>READ ONLY</literal> mode,
-   otherwise it is opened in <literal>READ WRITE</literal> mode.
-   (This rule is also applied to remote and local subtransactions.)
-  </para>
-
-  <para>
-   The remote transaction is also opened in the same deferrable mode as the
-   local transaction: if the local transaction is <literal>DEFERRABLE</literal>,
-   the remote transaction is opened in <literal>DEFERRABLE</literal> mode,
-   otherwise it is opened in <literal>NOT DEFERRABLE</literal> mode.
-  </para>
-
   <para>
    Note that it is currently not supported by
    <filename>postgres_fdw</filename> to prepare the remote transaction for
index 2e67e998adbc7b04ca95b5699ba23563c7301d62..b885513f76541bc165a41ce78f83623703c2410a 100644 (file)
@@ -1044,34 +1044,6 @@ 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
  */
index 7f11b91979941c1263c78dbeb48d7ba7bfbdade3..b2bc10ee04196465d4cbb0622892b75108e1bf75 100644 (file)
@@ -458,7 +458,6 @@ 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);