Skip to content

Commit 5076f88

Browse files
committed
Remove incidental dependencies on partitioned_rels lists.
It turns out that the calculation of [Merge]AppendPath.partitioned_rels in allpaths.c is faulty and sometimes omits relevant non-leaf partitions, allowing an assertion added by commit a929e17 to trigger. Rather than fix that, it seems better to get rid of those fields altogether. We don't really need the info until create_plan time, and calculating it once for the selected plan should be cheaper than calculating it for each append path we consider. This patch undoes a couple of very minor uses of the partitioned_rels values. createplan.c was testing for nil-ness to optimize away the preparatory work for make_partition_pruneinfo(). That is worth doing if the check is nigh free, but it's not worth going to any great lengths to avoid. create_append_path() was testing for nil-ness as part of deciding how to set up ParamPathInfo for an AppendPath. I replaced that with a check for the appendrel's parent rel being partitioned. That's not quite the same thing but should cover most cases. If we note any interesting loss of optimizations, we can dumb this down to just always use the more expensive method when the parent is a baserel. Discussion: https://postgr.es/m/87sg8tqhsl.fsf@aurora.ydns.eu Discussion: https://postgr.es/m/CAJKUy5gCXDSmFs2c=R+VGgn7FiYcLCsEFEuDNNLGfoha=pBE_g@mail.gmail.com
1 parent fb2d645 commit 5076f88

File tree

2 files changed

+9
-12
lines changed

2 files changed

+9
-12
lines changed

src/backend/optimizer/plan/createplan.c

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1227,8 +1227,7 @@ create_append_plan(PlannerInfo *root, AppendPath *best_path, int flags)
12271227
* pruning during execution. Gather information needed by the executor to
12281228
* do partition pruning.
12291229
*/
1230-
if (enable_partition_pruning &&
1231-
best_path->partitioned_rels != NIL)
1230+
if (enable_partition_pruning)
12321231
{
12331232
List *prunequal;
12341233

@@ -1393,8 +1392,7 @@ create_merge_append_plan(PlannerInfo *root, MergeAppendPath *best_path,
13931392
* pruning during execution. Gather information needed by the executor to
13941393
* do partition pruning.
13951394
*/
1396-
if (enable_partition_pruning &&
1397-
best_path->partitioned_rels != NIL)
1395+
if (enable_partition_pruning)
13981396
{
13991397
List *prunequal;
14001398

src/backend/optimizer/util/pathnode.c

Lines changed: 7 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1230,15 +1230,14 @@ create_append_path(PlannerInfo *root,
12301230

12311231
/*
12321232
* When generating an Append path for a partitioned table, there may be
1233-
* parameters that are useful so we can eliminate certain partitions
1234-
* during execution. Here we'll go all the way and fully populate the
1235-
* parameter info data as we do for normal base relations. However, we
1236-
* need only bother doing this for RELOPT_BASEREL rels, as
1237-
* RELOPT_OTHER_MEMBER_REL's Append paths are merged into the base rel's
1238-
* Append subpaths. It would do no harm to do this, we just avoid it to
1239-
* save wasting effort.
1233+
* parameterized quals that are useful for run-time pruning. Hence,
1234+
* compute path.param_info the same way as for any other baserel, so that
1235+
* such quals will be available for make_partition_pruneinfo(). (This
1236+
* would not work right for a non-baserel, ie a scan on a non-leaf child
1237+
* partition, and it's not necessary anyway in that case. Must skip it if
1238+
* we don't have "root", too.)
12401239
*/
1241-
if (partitioned_rels != NIL && root && rel->reloptkind == RELOPT_BASEREL)
1240+
if (root && rel->reloptkind == RELOPT_BASEREL && IS_PARTITIONED_REL(rel))
12421241
pathnode->path.param_info = get_baserel_parampathinfo(root,
12431242
rel,
12441243
required_outer);

0 commit comments

Comments
 (0)