Horton

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

amul sul <sulamul@gmail.com>

View thread
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-03-11T05:09:46Z
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 →

  1. Suppress unused-variable warning.

  2. Allow partitionwise joins in more cases.

  3. Avoid crash in partitionwise join planning under GEQO.

  4. Disable support for partitionwise joins in problematic cases.

  5. Add plan_cache_mode setting

  6. Add test for partitionwise join involving default partition.

  7. Revise API for partition_rbound_cmp/partition_rbound_datum_cmp.

On Mon, Mar 11, 2019 at 8:29 AM Ashutosh Bapat <ashutosh.bapat.oss@gmail.com>
wrote:

> On Thu, Mar 7, 2019 at 8:20 PM amul sul <sulamul@gmail.com> wrote:
>
>>
>>
>> On Thu, Mar 7, 2019 at 1:02 PM amul sul <sulamul@gmail.com> wrote:
>>
>>> Thanks Rajkumar,
>>>
>>> I am looking into this.
>>>
>>>
>> The crash happens when none of the if-else branch of
>> handle_missing_partition()
>> evaluates and returns merged_index unassigned.
>>
>> Let me explain, in Rajkumar 's test case, the join type is JOIN_INNER.
>> When
>> only outer rel has null partition, merge_null_partitions() function calls
>> handle_missing_partition() with missing_side_inner = false and
>> missing_side_outer = false
>>
>
> Both missing_side_ variables being false when the NULL partition is
> missing on the inner side looks suspicious. I guess from the variable names
> that the missing_side_inner should be true in this case.
>
>

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);



> 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?

Regards,
Amul