dummy relation in partitionwise join
Hi,
I was looking at:
commit 475dbd0b718de8ac44da144f934651b959e3b705
Author: David Rowley <drowley@postgresql.org>
Date: Tue Aug 3 11:47:24 2021 +1200
Track a Bitmapset of non-pruned partitions in RelOptInfo
I noticed that the dummy relation is skipped in the loop
over rel->live_parts.
I wonder if the following change is sensible.
Thanks
Attachments:
skip-adding-dummy.patchapplication/octet-stream; name=skip-adding-dummy.patchDownload
diff --git a/src/backend/optimizer/path/joinrels.c b/src/backend/optimizer/path/joinrels.c
index 16cc9269ef..9096017fea 100644
--- a/src/backend/optimizer/path/joinrels.c
+++ b/src/backend/optimizer/path/joinrels.c
@@ -1539,7 +1539,8 @@ try_partitionwise_join(PlannerInfo *root, RelOptInfo *rel1, RelOptInfo *rel2,
child_sjinfo,
child_sjinfo->jointype);
joinrel->part_rels[cnt_parts] = child_joinrel;
- joinrel->live_parts = bms_add_member(joinrel->live_parts, cnt_parts);
+ if (!IS_DUMMY_REL(child_joinrel))
+ joinrel->live_parts = bms_add_member(joinrel->live_parts, cnt_parts);
joinrel->all_partrels = bms_add_members(joinrel->all_partrels,
child_joinrel->relids);
}
On Fri, 29 Oct 2021 at 04:43, Zhihong Yu <zyu@yugabyte.com> wrote:
I noticed that the dummy relation is skipped in the loop over rel->live_parts.
I wonder if the following change is sensible.
I made the definition of live_parts to be partitions that survived
partition pruning. There's a few reasons why a RELOPT_BASEREL could
have dummy rels in its live_parts Bitmapset, so I don't see why we
should make an exception and remove them from RELOPT_OTHER_JOINREL.
If you think we should change the definition of what live_parts is,
then you'd need to come up with a patch that did it for everything.
However, I don't really think I'm for changing that. We currently set
live_parts for RELOPT_BASEREL just after we perform partition
pruning, so we could well have many bits set in that field for
relations that will become dummy rels later. At what point could code
looking at the live_parts field know that the bits that are set are
never dummy rels? Or would they always just have to check for dummy
rels just in case the bits are not unset for the dummy rels yet?
Seems a bit messy to me.
David
On Thu, Oct 28, 2021 at 2:19 PM David Rowley <dgrowleyml@gmail.com> wrote:
On Fri, 29 Oct 2021 at 04:43, Zhihong Yu <zyu@yugabyte.com> wrote:
I noticed that the dummy relation is skipped in the loop over
rel->live_parts.
I wonder if the following change is sensible.
I made the definition of live_parts to be partitions that survived
partition pruning. There's a few reasons why a RELOPT_BASEREL could
have dummy rels in its live_parts Bitmapset, so I don't see why we
should make an exception and remove them from RELOPT_OTHER_JOINREL.If you think we should change the definition of what live_parts is,
then you'd need to come up with a patch that did it for everything.However, I don't really think I'm for changing that. We currently set
live_parts for RELOPT_BASEREL just after we perform partition
pruning, so we could well have many bits set in that field for
relations that will become dummy rels later. At what point could code
looking at the live_parts field know that the bits that are set are
never dummy rels? Or would they always just have to check for dummy
rels just in case the bits are not unset for the dummy rels yet?
Seems a bit messy to me.David
Hi,
Thanks for the reply.
We can leave the code as is.
Cheers