Horton

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

Amit Langote <langote_amit_f8@lab.ntt.co.jp>

View thread View on postgr.es
From: Amit Langote <Langote_Amit_f8@lab.ntt.co.jp>
To: Ashutosh Bapat <ashutosh.bapat@enterprisedb.com>
Cc: Robert Haas <robertmhaas@gmail.com>, Etsuro Fujita <fujita.etsuro@lab.ntt.co.jp>, Thomas Munro <thomas.munro@enterprisedb.com>, Rafia Sabih <rafia.sabih@enterprisedb.com>, Rajkumar Raghuwanshi <rajkumar.raghuwanshi@enterprisedb.com>, pgsql-hackers <pgsql-hackers@postgresql.org>
Date: 2017-09-12T08:47:04Z
Lists: pgsql-hackers 
On 2017/09/12 16:39, Ashutosh Bapat wrote:
> On Tue, Sep 12, 2017 at 7:31 AM, Amit Langote
> <Langote_Amit_f8@lab.ntt.co.jp> wrote:
>> On 2017/09/11 19:45, Ashutosh Bapat wrote:
>>> On Mon, Sep 11, 2017 at 12:16 PM, Amit Langote wrote:
>>>> IMHO, we should make it the responsibility of the future patch to set a
>>>> child PlanRowMark's prti to the direct parent's RT index, when we actually
>>>> know that it's needed for something.  We clearly know today why we need to
>>>> pass the other objects like child RT entry, RT index, and Relation, so we
>>>> should limit this patch to pass only those objects to the recursive call.
>>>> That makes this patch a relatively easy to understand change.
>>>
>>> I think you are mixing two issues here 1. setting parent RTI in child
>>> PlanRowMark and 2. passing immediate parent's PlanRowMark to
>>> expand_single_inheritance_child().
>>>
>>> I have discussed 1 in my reply to Robert.
>>>
>>> About 2 you haven't given any particular comments to my reply. To me
>>> it looks like it's this patch that introduces the notion of
>>> multi-level expansion, so it's natural for this patch to pass
>>> PlanRowMark in cascaded fashion similar to other structures.
>>
>> You patch does 2 to be able to do 1, doesn't it?  That is, to be able to
>> set the child PlanRowMark's prti to the direct parent's RT index, you pass
>> the immediate parent's PlanRowMark to the recursive call of
>> expand_single_inheritance_child().
> 
> No. child PlanRowMark's prti is set to parentRTIndex, which is a
> separate argument and is used to also set parent_relid in
> AppendRelInfo.

OK.  So, to keep the old behavior (if at all), we'd actually need a new
argument rootParentRTindex.  Old behavior being that all child
PlanRowMarks has the rootParentRTindex as their prti.

It seems though that the new behavior where prti will now be set to the
direct parent's RT index is more or less harmless, because whatever we set
prti to, as long as it's different from rti, we can consider it a child
PlanRowMark.  So it might be fine to set prti to direct parent's RT index.

That said, I noticed that we might need to be careful about what the value
of the root parent's PlanRowMark's allMarkType field gets set to.  We need
to make sure that it reflects markType of all partitions in the tree,
including those that are not root parent's direct children.  Is that true
with the proposed implementation?

Thanks,
Amit

Commits

  1. Add test for partitionwise join involving default partition.

  2. Rewrite the code that applies scan/join targets to paths.

  3. Fix code related to partitioning schemes for dropped columns.

  4. Copy information from the relcache instead of pointing to it.

  5. Basic partition-wise join functionality.

  6. Associate partitioning information with each RelOptInfo.

  7. Expand partitioned table RTEs level by level, without flattening.

  8. Set partitioned_rels appropriately when UNION ALL is used.

  9. Remove dedicated B-tree root-split record types.

  10. Assorted preparatory refactoring for partition-wise join.

  11. Teach adjust_appendrel_attrs(_multilevel) to do multiple translations.

  12. Avoid unnecessary single-child Append nodes.

  13. Revisit handling of UNION ALL subqueries with non-Var output columns.