Track detached partitions more accurately in partdescs

In d6b8d29419df I (Álvaro) was sloppy about recording whether a
partition descripor does or does not include detached partitions, when
the snapshot checking does not see the pg_inherits row marked detached.
In that case no partition was omitted, yet in the relcache entry we were
saving the partdesc as omitting partitions.  Flip that (so we save it as
a partdesc not omitting partitions, which indeed it doesn't), which
hopefully makes the code easier to reason about.

Author: Amit Langote <amitlangote09@gmail.com>
Discussion: https://postgr.es/m/CA+HiwqE7GxGU4VdzwZzfiz+Ont5SsopoFkgtrZGEdPqWRL+biA@mail.gmail.com

diff --git a/src/backend/partitioning/partdesc.c b/src/backend/partitioning/partdesc.c
index fa9729283a05f694bdf290af29f99ee80944f1d8..cf1ca0fe5f75f55a2b83cf617e96cc1675dc2cbd 100644 (file)
--- a/src/backend/partitioning/partdesc.c
+++ b/src/backend/partitioning/partdesc.c
@@ -96,11 +96,11 @@ RelationGetPartitionDesc(Relation rel, bool omit_detached)
         */
        if (omit_detached &&
                rel->rd_partdesc_nodetached &&
-               TransactionIdIsValid(rel->rd_partdesc_nodetached_xmin) &&
                ActiveSnapshotSet())
        {
                Snapshot        activesnap;
 
+               Assert(TransactionIdIsValid(rel->rd_partdesc_nodetached_xmin));
                activesnap = GetActiveSnapshot();
 
                if (!XidInMVCCSnapshot(rel->rd_partdesc_nodetached_xmin, activesnap))
@@ -163,6 +163,7 @@ RelationBuildPartitionDesc(Relation rel, bool omit_detached)
                                                                                                 omit_detached, NoLock,
                                                                                                 &detached_exist,
                                                                                                 &detached_xmin);
+
        nparts = list_length(inhoids);
 
        /* Allocate working arrays for OIDs, leaf flags, and boundspecs. */
@@ -310,10 +311,17 @@ RelationBuildPartitionDesc(Relation rel, bool omit_detached)
        }
 
        /*
-        * Are we working with the partition that omits detached partitions, or
-        * the one that includes them?
+        * Are we working with the partdesc that omits the detached partition, or
+        * the one that includes it?
+        *
+        * Note that if a partition was found by the catalog's scan to have been
+        * detached, but the pg_inherit tuple saying so was not visible to the
+        * active snapshot (find_inheritance_children_extended will not have set
+        * detached_xmin in that case), we consider there to be no "omittable"
+        * detached partitions.
         */
-       is_omit = omit_detached && detached_exist && ActiveSnapshotSet();
+       is_omit = omit_detached && detached_exist && ActiveSnapshotSet() &&
+               TransactionIdIsValid(detached_xmin);
 
        /*
         * We have a fully valid partdesc.  Reparent it so that it has the right
@@ -343,9 +351,11 @@ RelationBuildPartitionDesc(Relation rel, bool omit_detached)
                 * For partdescs built excluding detached partitions, which we save
                 * separately, we also record the pg_inherits.xmin of the detached
                 * partition that was omitted; this informs a future potential user of
-                * such a cached partdescs.  (This might be InvalidXid; see comments
-                * in struct RelationData).
+                * such a cached partdesc to only use it after cross-checking that the
+                * xmin is indeed visible to the snapshot it is going to be working
+                * with.
                 */
+               Assert(TransactionIdIsValid(detached_xmin));
                rel->rd_partdesc_nodetached_xmin = detached_xmin;
        }
        else

diff --git a/src/include/utils/rel.h b/src/include/utils/rel.h
index 07ae2c70a9672d48276a763947413973630048f4..5bd44be9a79e0fda4b1b32b81f898a70559171b8 100644 (file)
--- a/src/include/utils/rel.h
+++ b/src/include/utils/rel.h
@@ -138,9 +138,7 @@ typedef struct RelationData
         * rd_partdesc_nodetached.  This informs a future user of that partdesc:
         * if this value is not in progress for the active snapshot, then the
         * partdesc can be used, otherwise they have to build a new one.  (This
-        * matches what find_inheritance_children_extended would do).  In the rare
-        * case where the pg_inherits tuple has been frozen, this will be
-        * InvalidXid; behave as if the partdesc is unusable in that case.
+        * matches what find_inheritance_children_extended would do).
         */
        TransactionId rd_partdesc_nodetached_xmin;