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