Update comments on rd_newRelfilenodeSubid.

Ensure comments accurately reflect state of code
given new understanding, and recent changes.
Include example code from Noah Misch to
illustrate how rd_newRelfilenodeSubid can be
reset deterministically. No code changes.

diff --git a/src/backend/commands/copy.c b/src/backend/commands/copy.c
index 4172c0cfbe68a8a9c6c960d1b40d1865766ada10..abd82cf9f59c8abddeb6a277f9bde469bc006528 100644 (file)
--- a/src/backend/commands/copy.c
+++ b/src/backend/commands/copy.c
@@ -1963,8 +1963,18 @@ CopyFrom(CopyState cstate)
         * routine first.
         *
         * As mentioned in comments in utils/rel.h, the in-same-transaction test
-        * is not completely reliable, since in rare cases rd_createSubid or
-        * rd_newRelfilenodeSubid can be cleared before the end of the transaction.
+        * is not always set correctly, since in rare cases rd_newRelfilenodeSubid
+        * can be cleared before the end of the transaction. The exact case is
+        * when a relation sets a new relfilenode twice in same transaction, yet
+        * the second one fails in an aborted subtransaction, e.g.
+        *
+        * BEGIN;
+        * TRUNCATE t;
+        * SAVEPOINT save;
+        * TRUNCATE t;
+        * ROLLBACK TO save;
+        * COPY ...
+        *
         * However this is OK since at worst we will fail to make the optimization.
         *
         * Also, if the target file is new-in-transaction, we assume that checking
@@ -1994,10 +2004,9 @@ CopyFrom(CopyState cstate)
                 * which subtransaction created it is crucial for correctness
                 * of this optimisation.
                 *
-                * Note that because the test is unreliable in case of relcache reset
-                * we cannot guarantee that we can honour the request to FREEZE.
-                * If we cannot honour the request we do so silently, firstly to
-                * avoid noise for the user and also to avoid obscure test failures.
+                * As noted above rd_newRelfilenodeSubid is not set in all cases
+                * where we can apply the optimization, so in those rare cases
+                * where we cannot honour the request we do so silently.
                 */
                if (cstate->freeze &&
                        ThereAreNoPriorRegisteredSnapshots() &&

diff --git a/src/include/utils/rel.h b/src/include/utils/rel.h
index f7f84e6f71e8e4c36e66dbb3b5b8614b846a0bae..ff5488a7d8b44b8ccf72bc1c3f655203ae93c6f8 100644 (file)
--- a/src/include/utils/rel.h
+++ b/src/include/utils/rel.h
@@ -90,11 +90,20 @@ typedef struct RelationData
        /*
         * rd_createSubid is the ID of the highest subtransaction the rel has
         * survived into; or zero if the rel was not created in the current top
-        * transaction.  This should be relied on only for optimization purposes;
-        * it is possible for new-ness to be "forgotten" (eg, after CLUSTER).
+        * transaction.  This can be now be relied on, whereas previously it
+        * could be "forgotten" in earlier releases.
         * Likewise, rd_newRelfilenodeSubid is the ID of the highest
         * subtransaction the relfilenode change has survived into, or zero if not
         * changed in the current transaction (or we have forgotten changing it).
+        * rd_newRelfilenodeSubid can be forgotten when a relation has multiple
+        * new relfilenodes within a single transaction, with one of them occuring
+        * in a subsequently aborted subtransaction, e.g.
+        *              BEGIN;
+        *              TRUNCATE t;
+        *              SAVEPOINT save;
+        *              TRUNCATE t;
+        *              ROLLBACK TO save;
+        *              -- rd_newRelfilenode is now forgotten
         */
        SubTransactionId rd_createSubid;        /* rel was created in current xact */
        SubTransactionId rd_newRelfilenodeSubid;        /* new relfilenode assigned in