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>, Robert Haas <robertmhaas@gmail.com>
Cc: 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-13T07:38:09Z
Lists: pgsql-hackers 
On 2017/09/13 16:21, Ashutosh Bapat wrote:
> On Wed, Sep 13, 2017 at 12:39 AM, Robert Haas <robertmhaas@gmail.com> wrote:
>> locks taken from the executor are worthless because plancache.c will
>> always do the job for us.  I don't know of a case where we execute a
>> saved plan without going through the plan cache, but that doesn't mean
>> that there isn't one or that there couldn't be one in the future.
>> It's not the job of these partitioning patches to whack around the way
>> we do locking in general -- they should preserve the existing behavior
>> as much as possible.  If we want to get rid of the locking in the
>> executor altogether, that's a separate discussion where, I have a
>> feeling, there will prove to be better reasons for the way things are
>> than we are right now supposing.
>>
> 
> I agree that it's not the job of these patches to change the locking
> or even get rid of partitioned_rels. In order to continue returning
> partitioned_rels in Append paths esp. in the case of queries involving
> set operations and partitioned table e.g "select 1 from t1 union all
> select 2 from t1;" in which t1 is multi-level partitioned table, we
> need a fix in add_paths_to_append_rels(). The fix provided in [1] is
> correct but we will need a longer explanation of why we have to
> involve RTE_SUBQUERY with RELKIND_PARTITIONED_TABLE. The explanation
> is complicated. If we get rid of partitioned_rels, we don't need to
> fix that code in add_paths_to_append_rel().

Yeah, let's get on with setting partitioned_rels in AppendPath correctly
in this patch.  Ashutosh's suggested approach seems fine, although it
needlessly requires to scan root->pcinfo_list.  But it shouldn't be longer
than the number of partitioned tables in the query, so maybe that's fine
too.  At least, it doesn't require us to add code to
add_paths_to_append_rel() that can be pretty hard to wrap one's head around.

That said, we might someday need to look carefully at some things that
Robert mentioned carefully, especially around the order of locks taken by
AcquireExecutorLocks() in light of the EIBO patch getting committed.

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.