Re: Wired if-statement in gen_partprune_steps_internal

From: Andy Fan <zhihui(dot)fan1213(at)gmail(dot)com>
To: Amit Langote <amitlangote09(at)gmail(dot)com>
Cc: David Rowley <dgrowleyml(at)gmail(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>
Subject: Re: Wired if-statement in gen_partprune_steps_internal
Date: 2021-04-12 07:58:11
Message-ID: CAKU4AWoF3fmoxohv2hv-zoWdNEpG9M8m6jnN=o90A+DQzy9MKg@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Thu, Apr 8, 2021 at 7:59 PM Amit Langote <amitlangote09(at)gmail(dot)com> wrote:

> On Thu, Apr 8, 2021 at 7:41 PM David Rowley <dgrowleyml(at)gmail(dot)com> wrote:
> > On Thu, 8 Apr 2021 at 21:04, Amit Langote <amitlangote09(at)gmail(dot)com>
> wrote:
> > > Maybe, we should also updated the description of node struct as
> > > follows to consider that last point:
> >>
> > > * PartitionPruneStepOp - Information to prune using a set of mutually
> ANDed
> > > * OpExpr and any IS [ NOT ] NULL clauses
> >
> > I didn't add that. I wasn't really sure if I understood why we'd talk
> > about PartitionPruneStepCombine in the PartitionPruneStepOp. I thought
> > the overview in gen_partprune_steps_internal was ok to link the two
> > together and explain why they're both needed.
>
> Sorry, maybe the way I wrote it was a bit confusing, but I meant to
> suggest that we do what I have quoted above from my last email. That
> is, we should clarify in the description of PartitionPruneStepOp that
> it contains information derived from OpExprs and in some cases also IS
> [ NOT ] NULL clauses.
>
> Thanks for the commit.
>
> --
> Amit Langote
> EDB: http://www.enterprisedb.com
>

Thanks for the patch.

Recently I am reading the partition prune code again, and want to
propose some tiny changes. That is helpful for me and hope it is
helpful for others as well, especially for the people who are not familiar
with these codes.

-- v1-0001-Document-enhancement-for-RelOptInfo.partexprs-nul.patch

Just add comments for RelOptInfo.partexprs & nullable_partexprs to
remind the reader nullable_partexprs is just for partition wise join. and
use bms_add_member(relinfo->all_partrels, childRTindex); instead of
bms_add_members(relinfo->all_partrels, childrelinfo->relids); which
would be more explicit to say add the child rt index to all_partrels.

-- v1-0002-Split-gen_prune_steps_from_exprs-into-some-smalle.patch

Just split the gen_prune_steps_from_opexps into some smaller chunks.
The benefits are the same as smaller functions.

--
Best Regards
Andy Fan (https://www.aliyun.com/)

Attachment Content-Type Size
v1-0001-Document-enhancement-for-RelOptInfo.partexprs-nul.patch application/octet-stream 2.2 KB
v1-0002-Split-gen_prune_steps_from_exprs-into-some-smalle.patch application/octet-stream 21.9 KB

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Christoph Berg 2021-04-12 08:09:39 Re: pgsql: Move tablespace path re-creation from the makefiles to pg_regres
Previous Message Amit Langote 2021-04-12 07:42:50 Re: ALTER TABLE .. DETACH PARTITION CONCURRENTLY