Re: [HACKERS] path toward faster partition pruning

From: Jesper Pedersen <jesper(dot)pedersen(at)redhat(dot)com>
To: David Rowley <david(dot)rowley(at)2ndquadrant(dot)com>, Amit Langote <Langote_Amit_f8(at)lab(dot)ntt(dot)co(dot)jp>, Alvaro Herrera <alvherre(at)alvh(dot)no-ip(dot)org>
Cc: Robert Haas <robertmhaas(at)gmail(dot)com>, Ashutosh Bapat <ashutosh(dot)bapat(at)enterprisedb(dot)com>, Rajkumar Raghuwanshi <rajkumar(dot)raghuwanshi(at)enterprisedb(dot)com>, Kyotaro HORIGUCHI <horiguchi(dot)kyotaro(at)lab(dot)ntt(dot)co(dot)jp>, Amit Langote <amitlangote09(at)gmail(dot)com>, Dilip Kumar <dilipbalaut(at)gmail(dot)com>, Beena Emerson <memissemerson(at)gmail(dot)com>, "pgsql-hackers(at)postgresql(dot)org" <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: [HACKERS] path toward faster partition pruning
Date: 2018-04-04 15:45:56
Message-ID: 3eedafaa-840f-bf72-2fa9-dadb0852e959@redhat.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Hi,

On 04/04/2018 09:29 AM, David Rowley wrote:
> Thanks for updating. I've made a pass over v49 and I didn't find very
> much wrong with it.
>
> The only real bug I found was a missing IsA(rinfo->clause, Const) in
> the pseudoconstant check inside
> generate_partition_pruning_steps_internal.
>
> Most of the changes are comment fixes with a few stylistic changes
> thrown which are pretty much all there just to try to shrink the code
> a line or two or reduce indentation.
>
> I feel pretty familiar with this code now and assuming the attached is
> included I'm happy for someone else, hopefully, a committer to take a
> look at it.
>
> I'll leave the following notes:
>
> 1. Still not sure about RelOptInfo->has_default_part. This flag is
> only looked at in generate_partition_pruning_steps. The RelOptInfo and
> the boundinfo is available to look at, it's just that the
> partition_bound_has_default macro is defined in partition.c rather
> than partition.h.
>
> 2. Don't really like the new isopne variable name. It's not very
> simple to decode, perhaps something like is_not_eq is better?
>
> 3. The part of the code I'm least familiar with is
> get_steps_using_prefix_recurse(). I admit to not having had time to
> fully understand that and consider ways to break it.
>
> Marking as ready for committer.
>

Passes check-world, and CommitFest app has been updated to reflect the
current patch set. Trivial changes attached.

Best regards,
Jesper

Attachment Content-Type Size
v49_fixes_jpedersen.patch text/x-patch 3.8 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Tom Lane 2018-04-04 16:19:34 Re: Add support for printing/reading MergeAction nodes
Previous Message Craig Ringer 2018-04-04 15:23:51 Re: PostgreSQL's handling of fsync() errors is unsafe and risks data loss at least on XFS