Re: Query with high planning time at version 11.1 compared versions 10.5 and 11.0

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