Revert "Fix partition pruning setup during DETACH CONCURRENTLY"
authorAlvaro Herrera <alvherre@alvh.no-ip.org>
Mon, 24 Jun 2024 15:20:21 +0000 (17:20 +0200)
committerAlvaro Herrera <alvherre@alvh.no-ip.org>
Mon, 24 Jun 2024 15:20:21 +0000 (17:20 +0200)
This reverts commit 27162a64b386; this branch is in code freeze due to a
nearing release.  We can commit again after the release is out.

Discussion: https://postgr.es/m/1158256.1719239648@sss.pgh.pa.us

src/backend/executor/execPartition.c

index c37a19a91f4256f02ea6512c9517af875868b7ac..bb14dcbe6fa85dcc30c243d3fed1f2861045e44a 100644 (file)
@@ -1942,31 +1942,37 @@ CreatePartitionPruneState(PlanState *planstate, PartitionPruneInfo *pruneinfo)
                        /*
                         * Initialize the subplan_map and subpart_map.
                         *
-                        * The set of partitions that exist now might not be the same that
-                        * existed when the plan was made.  The normal case is that it is;
-                        * optimize for that case with a quick comparison, and just copy
-                        * the subplan_map and make subpart_map point to the one in
-                        * PruneInfo.
+                        * Because we request detached partitions to be included, and
+                        * detaching waits for old transactions, it is safe to assume that
+                        * no partitions have disappeared since this query was planned.
                         *
-                        * For the case where they aren't identical, we could have more
-                        * partitions on either side; or even exactly the same number of
-                        * them on both but the set of OIDs doesn't match fully.  Handle
-                        * this by creating new subplan_map and subpart_map arrays that
-                        * corresponds to the ones in the PruneInfo where the new
-                        * partition descriptor's OIDs match.  Any that don't match can be
-                        * set to -1, as if they were pruned.  Both arrays must be in
-                        * numerical OID order.
+                        * However, new partitions may have been added.
                         */
+                       Assert(partdesc->nparts >= pinfo->nparts);
                        pprune->nparts = partdesc->nparts;
                        pprune->subplan_map = palloc(sizeof(int) * partdesc->nparts);
-
-                       if (partdesc->nparts == pinfo->nparts &&
-                               memcmp(partdesc->oids, pinfo->relid_map,
-                                          sizeof(int) * partdesc->nparts) == 0)
+                       if (partdesc->nparts == pinfo->nparts)
                        {
+                               /*
+                                * There are no new partitions, so this is simple.  We can
+                                * simply point to the subpart_map from the plan, but we must
+                                * copy the subplan_map since we may change it later.
+                                */
                                pprune->subpart_map = pinfo->subpart_map;
                                memcpy(pprune->subplan_map, pinfo->subplan_map,
                                           sizeof(int) * pinfo->nparts);
+
+                               /*
+                                * Double-check that the list of unpruned relations has not
+                                * changed.  (Pruned partitions are not in relid_map[].)
+                                */
+#ifdef USE_ASSERT_CHECKING
+                               for (int k = 0; k < pinfo->nparts; k++)
+                               {
+                                       Assert(partdesc->oids[k] == pinfo->relid_map[k] ||
+                                                  pinfo->subplan_map[k] == -1);
+                               }
+#endif
                        }
                        else
                        {
@@ -1974,18 +1980,22 @@ CreatePartitionPruneState(PlanState *planstate, PartitionPruneInfo *pruneinfo)
                                int                     pp_idx;
 
                                /*
-                                * When the partition arrays are not identical, there could be
-                                * some new ones but it's also possible that one was removed;
-                                * we cope with both situations by walking the arrays and
-                                * discarding those that don't match.
+                                * Some new partitions have appeared since plan time, and
+                                * those are reflected in our PartitionDesc but were not
+                                * present in the one used to construct subplan_map and
+                                * subpart_map.  So we must construct new and longer arrays
+                                * where the partitions that were originally present map to
+                                * the same sub-structures, and any added partitions map to
+                                * -1, as if the new partitions had been pruned.
                                 *
-                                * If the number of partitions on both sides match, it's still
-                                * possible that one partition has been detached and another
-                                * attached.  Cope with that by creating a map that skips any
-                                * mismatches.
+                                * Note: pinfo->relid_map[] may contain InvalidOid entries for
+                                * partitions pruned by the planner.  We cannot tell exactly
+                                * which of the partdesc entries these correspond to, but we
+                                * don't have to; just skip over them.  The non-pruned
+                                * relid_map entries, however, had better be a subset of the
+                                * partdesc entries and in the same order.
                                 */
                                pprune->subpart_map = palloc(sizeof(int) * partdesc->nparts);
-
                                for (pp_idx = 0; pp_idx < partdesc->nparts; pp_idx++)
                                {
                                        /* Skip any InvalidOid relid_map entries */
@@ -1993,7 +2003,6 @@ CreatePartitionPruneState(PlanState *planstate, PartitionPruneInfo *pruneinfo)
                                                   !OidIsValid(pinfo->relid_map[pd_idx]))
                                                pd_idx++;
 
-                       recheck:
                                        if (pd_idx < pinfo->nparts &&
                                                pinfo->relid_map[pd_idx] == partdesc->oids[pp_idx])
                                        {
@@ -2003,43 +2012,24 @@ CreatePartitionPruneState(PlanState *planstate, PartitionPruneInfo *pruneinfo)
                                                pprune->subpart_map[pp_idx] =
                                                        pinfo->subpart_map[pd_idx];
                                                pd_idx++;
-                                               continue;
                                        }
-
-                                       /*
-                                        * There isn't an exact match in the corresponding
-                                        * positions of both arrays.  Peek ahead in
-                                        * pinfo->relid_map to see if we have a match for the
-                                        * current partition in partdesc.  Normally if a match
-                                        * exists it's just one element ahead, and it means the
-                                        * planner saw one extra partition that we no longer see
-                                        * now (its concurrent detach finished just in between);
-                                        * so we skip that one by updating pd_idx to the new
-                                        * location and jumping above.  We can then continue to
-                                        * match the rest of the elements after skipping the OID
-                                        * with no match; no future matches are tried for the
-                                        * element that was skipped, because we know the arrays to
-                                        * be in the same order.
-                                        *
-                                        * If we don't see a match anywhere in the rest of the
-                                        * pinfo->relid_map array, that means we see an element
-                                        * now that the planner didn't see, so mark that one as
-                                        * pruned and move on.
-                                        */
-                                       for (int pd_idx2 = pd_idx + 1; pd_idx2 < pinfo->nparts; pd_idx2++)
+                                       else
                                        {
-                                               if (pd_idx2 >= pinfo->nparts)
-                                                       break;
-                                               if (pinfo->relid_map[pd_idx2] == partdesc->oids[pp_idx])
-                                               {
-                                                       pd_idx = pd_idx2;
-                                                       goto recheck;
-                                               }
+                                               /* this partdesc entry is not in the plan */
+                                               pprune->subplan_map[pp_idx] = -1;
+                                               pprune->subpart_map[pp_idx] = -1;
                                        }
-
-                                       pprune->subpart_map[pp_idx] = -1;
-                                       pprune->subplan_map[pp_idx] = -1;
                                }
+
+                               /*
+                                * It might seem that we need to skip any trailing InvalidOid
+                                * entries in pinfo->relid_map before checking that we scanned
+                                * all of the relid_map.  But we will have skipped them above,
+                                * because they must correspond to some partdesc->oids
+                                * entries; we just couldn't tell which.
+                                */
+                               if (pd_idx != pinfo->nparts)
+                                       elog(ERROR, "could not match partition child tables to plan elements");
                        }
 
                        /* present_parts is also subject to later modification */