Amit-san,
(2019/01/15 10:46), Amit Langote wrote:
On 2019/01/11 20:04, Etsuro Fujita wrote:
(2019/01/11 13:46), Amit Langote wrote:
However, what you proposed here as-is would
not keep that behavior, because rel->part_scheme is created for those
tables as well
(even though there would be no need IIUC).
Partition pruning uses part_scheme and pruning can occur even if a
partitioned table is under UNION ALL, so it *is* needed in that case.
Ah, you are right. Thanks for pointing that out!
I think
enable_partitionwise_join should only be checked in relnode.c or
joinrels.c.
Sorry, I don't understand this.
What I was trying to say is that we should check the GUC close to where
partitionwise join is actually implemented even though there is no such
hard and fast rule. Or maybe I'm just a bit uncomfortable with setting
consider_partitionwise_join based on the GUC.
I didn't think so. Consider the consider_parallel flag. I think the
way of setting it deviates from that rule already; it is set essentially
based on a GUC and is set in set_base_rel_sizes() (ie, before
implementing parallel paths). When adding the
consider_partitionwise_join flag, I thought it would be a good idea to
set consider_partitionwise_join in a similar way to consider_parallel,
keeping build_simple_rel() simple.
I've attached a patch to show what I mean. Can you please
take a look?
Thanks for the patch! Maybe I'm missing something, but I don't have a
strong opinion about that change. I'd rather think to modify
build_simple_rel so that it doesn't create rel->part_scheme if unnecessary
(ie, partitioned tables contained in inheritance trees where the top
parent is a UNION ALL subquery).
As I said above, partition pruning can occur even if a partitioned table
happens to be under UNION ALL. However, we *can* avoid creating
part_scheme and setting other partitioning properties if *all* of
enable_partition_pruning, enable_partitionwise_join, and
enable_partitionwise_aggregate are turned off.
Yeah, I think so.
If you think that this patch is a good idea, then you'll need to
explicitly set consider_partitionwise_join to false for a dummy partition
rel in set_append_rel_size(), because the assumption of your patch that
such partition's rel's consider_partitionwise_join would be false (as
initialized with the current code) would be broken by my patch. But that
might be a good thing to do anyway as it will document the special case
usage of consider_partitionwise_join variable more explicitly, assuming
you'll be adding a comment describing why it's being set to false
explicitly.
I'm not sure we need this as part of a fix for the issue reported on this
thread. I don't object to what you proposed here, but that would be
rather an improvement, so I think we should leave that for another patch.
Sure, no problem with committing it separately if at all.
OK
Thanks!
Best regards,
Etsuro Fujita
