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

On Wed, Apr 24, 2019 at 4:56 PM amul sul <sulamul(at)gmail(dot)com> wrote:

> Attached version is rebase atop of the latest master head(fdc7efcc30), also
> incorporates the Ashutosh's suggestion, thanks.
>
Thanks for rebase patch, patches applied cleanly on PG head.
I did some crash testing with extra test case [0006 patch] [1] and found no
more issue.

Thanks & Regards,
Rajkumar Raghuwanshi
QMG, EnterpriseDB.

[1]
https://www.postgresql.org/message-id/CAFjFpReKuV_4LRRfdy80BqX8oZfwbo%2BHWLQNv3CsJ5iGPSyTfA%40mail.gmail.com
<https://www.postgresql.org/message-id/CA%2Bq6zcU3X4%3DBfqnWXAUPBFtKK7vy0HO7-%2BmAW6KB2Zy_EPtC_Q%40mail.gmail.com>

>
> Regards,
> Amul
>
> On Mon, Mar 11, 2019 at 10:14 PM Ashutosh Bapat <
> ashutosh(dot)bapat(dot)oss(at)gmail(dot)com> wrote:
>
>>
>>
>> On Mon, Mar 11, 2019 at 10:40 AM amul sul <sulamul(at)gmail(dot)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
>>
>