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

On Tue, Dec 5, 2017 at 1:24 PM, Rajkumar Raghuwanshi
<rajkumar.raghuwanshi@enterprisedb.com> wrote:
> On Tue, Dec 5, 2017 at 11:04 AM, Rajkumar Raghuwanshi
> <rajkumar.raghuwanshi@enterprisedb.com> wrote:
>> On Mon, Dec 4, 2017 at 7:34 AM, Ashutosh Bapat
>> <ashutosh.bapat@enterprisedb.com> wrote:
>>> I agree, the patch looks longer than expected. I think, it's important
>>> to have some testcases to test partition-wise join with default
>>> partitions. I think we need at least one test for range default
>>> partitions, one test for list partitioning, one for multi-level
>>> partitioning and one negative testcase with default partition missing
>>> from one side of join.
>>>
>>> May be we could reduce the number of SQL commands and queries in the
>>> patch by adding default partition to every table that participates in
>>> partition-wise join (leave the tables participating in negative tests
>>> aside.). But that's going to increase the size of EXPLAIN outputs and
>>> query results. The negative test may simply drop the default partition
>>> from one of the tables.
>>>
>>> For every table being tested, the patch adds two ALTER TABLE commands,
>>> one for detaching an existing partition and then attach the same as
>>> default partition. Alternative to that is just add a new default
>>> partition without detaching and existing partition. But then the
>>> default partition needs to populated with some data, which requires 1
>>> INSERT statement at least. That doesn't reduce the size of patch, but
>>> increases the output of query and EXPLAIN plan.
>>>
>>> May be in case of multi-level partitioning test, we don't need to add
>>> DEFAULT in every partitioned relation; adding to one of them would be
>>> enough. May be add it to the parent, but that too can be avoided. That
>>> would reduce the size of patch a bit.
>>
>> Thanks Ashutosh for suggestions.
>>
>> I have reduced test cases as suggested. Attaching updated patch.
>>
> Sorry Attached wrong patch.
>
> attaching correct patch now.

Thanks. Here are some comments

+-- test default partition behavior for range
+ALTER TABLE prt1 DETACH PARTITION prt1_p3;
+ALTER TABLE prt1 ATTACH PARTITION prt1_p3 DEFAULT;
+ALTER TABLE prt2 DETACH PARTITION prt2_p3;
+ALTER TABLE prt2 ATTACH PARTITION prt2_p3 DEFAULT;

I think we need an ANALYZE here in case the statistics gets updated while
DETACH and ATTACH is going on. Other testcases also need to be updated with
ANALYZE, including the negative one.

+-- partition-wise join can not be applied if the only one of joining table have

Correction: ... if only one of the joining tables has ...

Please add the patch to the next commitfest so that it's not
forgotten. I think we can get rid of the multi-level partition-wise
testcase as well. Also, since we are re-attaching existing partition
tables as default partitions, we don't need to check the output as
well; just plan should be enough.

-- 
Best Wishes,
Ashutosh Bapat
EnterpriseDB Corporation
The Postgres Database Company