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

From: Dmitry Dolgov <9erthalion6(at)gmail(dot)com>
To: Ashutosh Bapat <ashutosh(dot)bapat(at)enterprisedb(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-03-21 23:02:02
Message-ID: CA+q6zcVCu2e1-ckF6+UjrXCt_qXt+jUq5JXqJOK-sMAEuq1Phw@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

> On 12 March 2018 at 06:00, Ashutosh Bapat <ashutosh(dot)bapat(at)enterprisedb(dot)com> wrote:
> Thanks for the note. Here are rebased patches.

Since I started to look at this patch, I can share few random notes (although
it's not a complete review, I'm in the progress now), maybe it can be helpful.

In `partition_range_bounds_merge`

+ if (!merged)
+ break;

is a bit redundant I think, because every time `merged` set to false it
followed by break.

At the end same function

+ if (merged)
+ merged = merge_default_partitions(outer_bi, outer_pmap, outer_mmap,
+ inner_bi, inner_pmap, inner_mmap,
+ jointype, &next_index,
+ &default_index);

Looks like I misunderstand something in the algorithm, but aren't default
partitions were already processed before e.g. here:

+ /*
+ * Default partition on the outer side forms the default
+ * partition of the join result.
+ */

Also in here

+ /* Free any memory we used in this function. */
+ list_free(merged_datums);
+ list_free(merged_indexes);
+ pfree(outer_pmap);
+ pfree(inner_pmap);
+ pfree(outer_mmap);
+ pfree(inner_mmap);

I think `merged_kinds` is missing.

I've noticed that in some places `IS_PARTITIONED_REL` was replaced

- if (!IS_PARTITIONED_REL(joinrel))
+ if (joinrel->part_scheme == NULL)

but I'm not quite follow why? Is it because `boundinfo` is not available
anymore at this point? If so, maybe it makes sense to update the commentary for
this macro and mention to not use for joinrel.

Also, as for me it would be helpful to have some commentary about this new
`partsupfunc`, what is exactly the purpose of it (but it's maybe just me
missing some obvious things from partitioning context)

+ FmgrInfo *partsupfunc;

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Thomas Munro 2018-03-21 23:07:57 Re: JIT compiling with LLVM v12.2
Previous Message Peter Eisentraut 2018-03-21 22:50:00 Re: Jsonb transform for pl/python