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

From: Ashutosh Bapat <ashutosh(dot)bapat(at)enterprisedb(dot)com>
To: Dmitry Dolgov <9erthalion6(at)gmail(dot)com>
Cc: Robert Haas <robertmhaas(at)gmail(dot)com>, Amit Langote <Langote_Amit_f8(at)lab(dot)ntt(dot)co(dot)jp>, Antonin Houska <ah(at)cybertec(dot)at>, pgsql-hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: [HACKERS] advanced partition matching algorithm for partition-wise join
Date: 2018-05-14 11:14:23
Message-ID: CAFjFpRfx7XHF_Pn-6_zSSCmrHQ9GHROt3GqmKzJeqW4e77Auiw@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Thanks Dmitry for your review.

On Mon, May 7, 2018 at 1:06 AM, Dmitry Dolgov <9erthalion6(at)gmail(dot)com> wrote:
>
> Thank you. Unfortunately, there are already few more conflicts since last time,
> could you rebase again?

Done.

>
> In the meantime, I'm a bit confused by `merge_null_partitions` and why
> partitions with NULL values should be treated separately? It became even more
> confusing when I looked at where this functions is used:
>
> /* If merge is unsuccessful, bail out without any further processing. */
> if (merged)
> merged = merge_null_partitions(outer_bi, outer_pmap, outer_mmap,
> inner_bi, inner_pmap, inner_mmap,
> jointype, &next_index, &null_index,
> &default_index);
>
> Is this commentary correct?

I agree. It's misleading. Removed.

>
> Also, I don't see anywhere in PostgreSQL itself or in this patch a definition
> of the term "NULL partition", can you add it, just to make things clear?

By NULL partition, I mean a list partition which holds NULL values.
Added that clarification in the prologue of merge_null_partitions().

>
> Another question, I see this pattern a lot here when there is a code like:
>
> if (!outer_has_something && inner_has_something)
> {
> // some logic
> }
> else if (outer_has_something && !inner_has_something)
> {
> // some logic symmetric to what we have above
> }
>
> By symmetric I mean that the code is more or less the same, just "inner"
> variables were changed to "outer" (and required types of joins are opposite).
> Is it feasible to actually use completely the same logic, and just change
> "inner"/"outer" variables instead?

I have added handle_missing_partition() for the same purpose. But I
didn't use it in merge_null_partitions(), which I have done in the
attached patches. Now the number of such instances are just at two
places one in merge_default_partitions() and the other in
handle_missing_partition(). But they are different enough not to
extract into a common function.

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

Attachment Content-Type Size
pg_adv_dp_join_patches_v9.tar.gz application/x-gzip 152.5 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Stas Kelvich 2018-05-14 11:20:59 Re: Global snapshots
Previous Message David Steele 2018-05-14 11:10:18 Re: Postgres 11 release notes