Re: generic plans and "initial" pruning

From: Amit Langote <amitlangote09(at)gmail(dot)com>
To: Robert Haas <robertmhaas(at)gmail(dot)com>
Cc: Jacob Champion <jchampion(at)timescale(dot)com>, Zhihong Yu <zyu(at)yugabyte(dot)com>, David Rowley <dgrowleyml(at)gmail(dot)com>, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: generic plans and "initial" pruning
Date: 2022-10-27 02:41:55
Message-ID: CA+HiwqGhT88L9k4-B1iEjf7H3fE=0ffWAJOfZ-6rmM3wwDU67g@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Mon, Oct 17, 2022 at 6:29 PM Amit Langote <amitlangote09(at)gmail(dot)com> wrote:
> On Wed, Oct 12, 2022 at 4:36 PM Amit Langote <amitlangote09(at)gmail(dot)com> wrote:
> > On Fri, Jul 29, 2022 at 1:20 PM Amit Langote <amitlangote09(at)gmail(dot)com> wrote:
> > > On Thu, Jul 28, 2022 at 1:27 AM Robert Haas <robertmhaas(at)gmail(dot)com> wrote:
> > > > 0001 adds es_part_prune_result but does not use it, so maybe the
> > > > introduction of that field should be deferred until it's needed for
> > > > something.
> > >
> > > Oops, looks like a mistake when breaking the patch. Will move that bit to 0002.
> >
> > Fixed that and also noticed that I had defined PartitionPruneResult in
> > the wrong header (execnodes.h). That led to PartitionPruneResult
> > nodes not being able to be written and read, because
> > src/backend/nodes/gen_node_support.pl doesn't create _out* and _read*
> > routines for the nodes defined in execnodes.h. I moved its definition
> > to plannodes.h, even though it is not actually the planner that
> > instantiates those; no other include/nodes header sounds better.
> >
> > One more thing I realized is that Bitmapsets added to the List
> > PartitionPruneResult.valid_subplan_offs_list are not actually
> > read/write-able. That's a problem that I also faced in [1], so I
> > proposed a patch there to make Bitmapset a read/write-able Node and
> > mark (only) the Bitmapsets that are added into read/write-able node
> > trees with the corresponding NodeTag. I'm including that patch here
> > as well (0002) for the main patch to work (pass
> > -DWRITE_READ_PARSE_PLAN_TREES build tests), though it might make sense
> > to discuss it in its own thread?
>
> Had second thoughts on the use of List of Bitmapsets for this, such
> that the make-Bitmapset-Nodes patch is no longer needed.
>
> I had defined PartitionPruneResult such that it stood for the results
> of pruning for all PartitionPruneInfos contained in
> PlannedStmt.partPruneInfos (covering all Append/MergeAppend nodes that
> can use partition pruning in a given plan). So, it had a List of
> Bitmapset. I think it's perhaps better for PartitionPruneResult to
> cover only one PartitionPruneInfo and thus need only a Bitmapset and
> not a List thereof, which I have implemented in the attached updated
> patch 0002. So, instead of needing to pass around a
> PartitionPruneResult with each PlannedStmt, this now passes a List of
> PartitionPruneResult with an entry for each in
> PlannedStmt.partPruneInfos.

Rebased over 3b2db22fe.

--
Thanks, Amit Langote
EDB: http://www.enterprisedb.com

Attachment Content-Type Size
v23-0001-Move-PartitioPruneInfo-out-of-plan-nodes-into-Pl.patch application/octet-stream 17.2 KB
v23-0002-Optimize-AcquireExecutorLocks-by-locking-only-un.patch application/octet-stream 82.3 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Justin Pryzby 2022-10-27 02:49:34 Re: GUC values - recommended way to declare the C variables?
Previous Message shiy.fnst@fujitsu.com 2022-10-27 02:34:24 RE: Perform streaming logical transactions by background workers and parallel apply