Re: [HACKERS] advanced partition matching algorithm for partition-wise join

amul sul <sulamul@gmail.com>

From: amul sul <sulamul@gmail.com>

To: Ashutosh Bapat <ashutosh.bapat.oss@gmail.com>

Cc: Rajkumar Raghuwanshi <rajkumar.raghuwanshi@enterprisedb.com>, Etsuro Fujita <fujita.etsuro@lab.ntt.co.jp>, Dmitry Dolgov <9erthalion6@gmail.com>, Thomas Munro <thomas.munro@enterprisedb.com>, Amit Langote <Langote_Amit_f8@lab.ntt.co.jp>, Antonin Houska <ah@cybertec.at>, Robert Haas <robertmhaas@gmail.com>, PostgreSQL-development <pgsql-hackers@postgresql.org>

Date: 2019-04-24T11:26:06Z

Lists: pgsql-hackers

Commits

Same data as JSON: GET /api/v1/messages/:b64id/commits the thread's linked commits as JSON, with link sources. API reference →

Suppress unused-variable warning.
- 401418ca6a68 13.0 landed
Allow partitionwise joins in more cases.
- c8434d64ce03 13.0 landed
Avoid crash in partitionwise join planning under GEQO.
- 7ad6498fd5a6 12.0 cited
- d70c147fa217 11.3 cited
Disable support for partitionwise joins in problematic cases.
- 7cfdc77023ad 12.0 cited
Add plan_cache_mode setting
- f7cb2842bf47 12.0 cited
Add test for partitionwise join involving default partition.
- 4513d3a4be0b 12.0 cited
Revise API for partition_rbound_cmp/partition_rbound_datum_cmp.
- b0229235564f 11.0 landed

Attachments

0002-Partition-wise-join-for-1-1-1-0-0-1-partition-matchi-v21.patch (application/octet-stream) patch v21-0002
0001-Hash-partition-bound-equality-refactoring-v21.patch (application/octet-stream) patch v21-0001
0003-Tests-for-0-1-1-1-and-1-0-partition-matching-v21.patch (application/octet-stream) patch v21-0003

Attached version is rebase atop of the latest master head(fdc7efcc30), also
incorporates the Ashutosh's suggestion, thanks.

Regards,
Amul

On Mon, Mar 11, 2019 at 10:14 PM Ashutosh Bapat <
ashutosh.bapat.oss@gmail.com> wrote:

>
>
> On Mon, Mar 11, 2019 at 10:40 AM amul sul <sulamul@gmail.com> wrote:
>
>>
>> All the places from where this handle_missing_partition() get called
>> have the following code to decide the value for missing_side_outer/_inner
>> which
>> I yet to understand. Do you think this has some flaw?
>>
>>         /*
>>          * For a FULL join, inner relation acts as both OUTER and INNER
>>          * relation.  For LEFT and ANTI join the inner relation acts as
>>          * INNER relation. For INNER and SEMI join OUTER and INNER
>>          * differentiation is immaterial.
>>          */
>>         missing_side_inner = (jointype == JOIN_FULL ||
>>                               jointype == JOIN_LEFT ||
>>                               jointype == JOIN_ANTI);
>>         missing_side_outer = (jointype == JOIN_FULL);
>>
>
> I was wrong, sorry. The comment says it all.
>
>
>>
>>
>>
>>> argument value which fails to set merged_index.
>>>>
>>>> In the attached patch, I tried to fix this case by setting merged_index
>>>> explicitly which fixes the reported crash.
>>>>
>>>
>>> I expect handle_missing_partition() to set the merged_index always. In
>>> your patches, I don't see that function in your patches is setting it
>>> explicitly. If we are setting merged_index explicitly somewhere else, other
>>> places may miss that explicit assignment. So it's better to move it inside
>>> this function.
>>>
>>>
>>
>> Ok, that can be fixed.
>>
>> Similarly, I think merge_null_partitions should set null_index instead of
>> asserting when null partitions missing from both the side, make sense?
>>
>
> I think not. null_index, once set shouldn't change and hence does not
> change with each pair of partitions being matched. So, it makes sense to
> make sure that null_index remains invalid if none of the tables have null
> partition.
>
> --
> Best Wishes,
> Ashutosh Bapat
>