Re: Partition-wise join for join between (declaratively) partitioned tables

On Fri, Sep 8, 2017 at 1:47 AM, Amit Langote
<Langote_Amit_f8@lab.ntt.co.jp> wrote:
> When I tried the attached patch, it doesn't seem to expand partitioning
> inheritance in step-wise manner as the patch's title says.  I think the
> rewritten patch forgot to include Ashutosh's changes to
> expand_single_inheritance_child() whereby the AppendRelInfo of the child
> will be marked with the direct parent instead of always the root parent.

Woops.

> I updated the patch to include just those changes.  I'm not sure about
> one of the Ashutosh's changes whereby the child PlanRowMark is also passed
> to expand_partitioned_rtentry() to use as the parent PlanRowMark.  I think
> the child RTE, child RT index and child Relation are fine, because they
> are necessary for creating AppendRelInfos in a desired way for later
> planning steps.  But PlanRowMarks are not processed within the planner
> afterwards and do not need to be marked with the immediate parent-child
> association in the same way that AppendRelInfos need to be.

We probably need some better comments to explain which things need to
be marked using the immediate parent and which need to be marked using
the baserel, and why.

> I also included the changes to add_paths_to_append_rel() from my patch on
> the "path toward faster partition pruning" thread.  We'd need that change,
> because while add_paths_to_append_rel() is called for all partitioned
> table RTEs in a given partition tree, expand_inherited_rtentry() would
> have set up a PartitionedChildRelInfo only for the root parent, so
> get_partitioned_child_rels() would not find the same for non-root
> partitioned table rels and crash failing the Assert.  The changes I made
> are such that we call get_partitioned_child_rels() only for the parent
> rels that are known to correspond root partitioned tables (or as you
> pointed out on the thread, "the table named in the query" as opposed those
> added to the query as result of inheritance expansion).  In addition to
> the relkind check on the input RTE, it would seem that checking that the
> reloptkind is RELOPT_BASEREL would be enough.  But actually not, because
> if a partitioned table is accessed in a UNION ALL query, reloptkind even
> for the root partitioned table (the table named in the query) would be
> RELOPT_OTHER_MEMBER_REL.  The only way to confirm that the input rel is
> actually the root partitioned table is to check whether its parent rel is
> not RTE_RELATION, because the parent in case of UNION ALL Append is a
> RTE_SUBQUERY RT entry.

OK, so this needs some good comments, too...

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company