Re: speeding up planning with partitions

On 2019/04/23 7:08, Tom Lane wrote:
> Amit Langote <Langote_Amit_f8@lab.ntt.co.jp> writes:
>> On 2019/04/02 2:34, Tom Lane wrote:
>>> I'm not at all clear on what we think the interaction between
>>> enable_partition_pruning and constraint_exclusion ought to be,
>>> so I'm not sure what the appropriate resolution is here.  Thoughts?
> 
>> Prior to 428b260f87 (that is, in PG 11), partition pruning for UPDATE and
>> DELETE queries is realized by applying constraint exclusion to the
>> partition constraint of the target partition.  The conclusion of the
>> discussion when adding the enable_partition_pruning GUC [1] was that
>> whether or not constraint exclusion is carried out (to facilitate
>> partition pruning) should be governed by the new GUC, not
>> constraint_exclusion, if only to avoid confusing users.
> 
> I got back to thinking about how this ought to work.

Thanks a lot for taking time to look at this.

> It appears to me
> that we've got half a dozen different behaviors that depend on one or both
> of these settings:
> 
> 1. Use of ordinary table constraints (CHECK, NOT NULL) in baserel pruning,
>   that is relation_excluded_by_constraints for baserels.
>   This is enabled by constraint_exclusion = on.
> 
> 2. Use of partition constraints in baserel pruning (applicable only
>   when a partition is accessed directly).
>   This is currently partly broken, and it's what your patch wants to
>   change.

Yes.  Any fix we come up with for this will need to be back-patched to 11,
because it's a regression introduced in 11 when the then new partition
pruning feature was committed (9fdb675fc).

> 3. Use of ordinary table constraints in appendrel pruning,
>   that is relation_excluded_by_constraints for appendrel members.
>   This is enabled by constraint_exclusion >= partition.
> 
> 4. Use of partition constraints in appendrel pruning.
>   This is enabled by the combination of enable_partition_pruning AND
>   constraint_exclusion >= partition.  However, it looks to me like this
>   is now nearly if not completely useless because of #5.
> 
> 5. Use of partition constraints in expand_partitioned_rtentry.
>   Enabled by enable_partition_pruning (see prune_append_rel_partitions).

Right, #5 obviates #4.

> 6. Use of partition constraints in run-time partition pruning.
>   This is also enabled by enable_partition_pruning, cf
>   create_append_plan, create_merge_append_plan.
> 
> Now in addition to what I mention above, there are assorted random
> differences in behavior depending on whether we are in an inherited
> UPDATE/DELETE or not.  I consider these differences to be so bogus
> that I'm not even going to include them in this taxonomy; they should
> not exist.  The UPDATE/DELETE target ought to act the same as a baserel.

The *partition* constraint of UPDATE/DELETE targets would never be refuted
by the query, because we process only those partition targets that remain
after applying partition pruning during the initial planning of the query
as if it were SELECT.  I'm saying we should distinguish such targets as
such when addressing #2.

Not sure if you'll like it but maybe we could ignore even regular
inheritance child targets that are proven to be empty (is_dummy_rel()) for
a given query during the initial SELECT planning.  That way, we can avoid
re-running relation_excluded_by_constraints() a second time for *all*
child target relations.

> I think this is ridiculously overcomplicated even without said random
> differences.  I propose that we do the following:
> 
> * Get rid of point 4 by not considering partition constraints for
> appendrel members in relation_excluded_by_constraints.  It's just
> useless cycles in view of point 5, or nearly so.  (Possibly there
> are corner cases where we could prove contradictions between a
> relation's partition constraints and regular constraints ... but is
> it really worth spending planner cycles to look for that?)

I guess not.  If partition constraint contradicts regular constraints,
there wouldn't be any data in such partitions to begin with, no?

> * Make point 2 like point 1 by treating partition constraints for
> baserels like ordinary table constraints, ie, they are considered
> only when constraint_exclusion = on (independently of whether
> enable_partition_pruning is on).

Right, enable_partition_pruning should only apply to appendrel pruning.
If a partition is accessed directly and hence a baserel to the planner, we
only consider constraint_exclusion and perform it only if the setting is on.

Another opinion on this is that we treat partition constraints differently
from regular constraints and don't mind the setting of
constraint_exclusion, that is, always perform constraint exclusion using
partition constraints.

> * Treat an inherited UPDATE/DELETE target table as if it were an
> appendrel member for the purposes of relation_excluded_by_constraints,
> thus removing the behavioral differences between SELECT and UPDATE/DELETE.

As I mentioned above, planner encounters any given UPDATE/DELETE *child*
target *twice*.  Once during the initial SELECT planning and then again
during when planning the query with a given child target relation as its
resultRelation.  For partition targets, since the initial run only selects
those that survive pruning, their partition constraint need not be
considered in the 2nd encounter as the query's baserel.

Also, it's during the 2nd encounter that we check inhTargetKind setting to
distinguish partition target baserels from SELECT baserels.  Its value is
INHKIND_INHERITED or INHKIND_PARTITIONED for the former, whereas it's
INHKIND_NONE for the latter.

> With this, constraint_exclusion would act pretty much as it traditionally
> has, and in most cases would not have any special impact on partitions
> compared to old-style inheritance.  The behaviors that
> enable_partition_pruning would control are expand_partitioned_rtentry
> pruning and run-time pruning, neither of which have any applicability to
> old-style inheritance.

That's right.

Do you want me to update my patch considering the above summary?

Thanks,
Amit