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

From: Ashutosh Bapat <ashutosh(dot)bapat(dot)oss(at)gmail(dot)com>
To: amul sul <sulamul(at)gmail(dot)com>
Cc: Rajkumar Raghuwanshi <rajkumar(dot)raghuwanshi(at)enterprisedb(dot)com>, Etsuro Fujita <fujita(dot)etsuro(at)lab(dot)ntt(dot)co(dot)jp>, Dmitry Dolgov <9erthalion6(at)gmail(dot)com>, Thomas Munro <thomas(dot)munro(at)enterprisedb(dot)com>, Amit Langote <Langote_Amit_f8(at)lab(dot)ntt(dot)co(dot)jp>, Antonin Houska <ah(at)cybertec(dot)at>, Robert Haas <robertmhaas(at)gmail(dot)com>, PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: [HACKERS] advanced partition matching algorithm for partition-wise join
Date: 2019-03-11 02:58:54
Message-ID: CAExHW5v6ir7y0eZe5qKBX87=Xbv2qZs2ddavkMRfWPeQTG2k7w@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Thu, Mar 7, 2019 at 8:20 PM amul sul <sulamul(at)gmail(dot)com> wrote:

>
>
> On Thu, Mar 7, 2019 at 1:02 PM amul sul <sulamul(at)gmail(dot)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.

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

--
Best Wishes,
Ashutosh Bapat

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Tomas Vondra 2019-03-11 02:59:33 Re: [HACKERS] PATCH: multivariate histograms and MCV lists
Previous Message Amit Kapila 2019-03-11 02:51:21 Re: pgsql: tableam: introduce table AM infrastructure.