public inbox for [email protected]
help / color / mirror / Atom feedFrom: David Rowley <[email protected]>
To: Amit Langote <[email protected]>
Cc: Robert Haas <[email protected]>
Cc: Tom Lane <[email protected]>
Cc: PostgreSQL-development <[email protected]>
Subject: Re: generic plans and "initial" pruning
Date: Fri, 8 Apr 2022 00:41:13 +1200
Message-ID: <CAApHDvroRU6wZC-Ruo5jQjEWOgcdey=gU0jwp-W2_S6DUVR8Bg@mail.gmail.com> (raw)
In-Reply-To: <CA+HiwqH4qQ_YVROr7TY0jSCuGn0oHhH79_DswOdXWN5UnMCBtQ@mail.gmail.com>
References: <CA+HiwqFGkMSge6TgC9KQzde0ohpAycLQuV7ooitEEpbKB0O_mg@mail.gmail.com>
<CA+TgmoZ=92LxPeVJ20vf4vJ4o8dr7Q2seDQR=xLY04puLjDs_A@mail.gmail.com>
<CA+HiwqEAdv+1t+v6pkcH9pgsSXnBeHLchDuGRwXP-oEB4BCPmg@mail.gmail.com>
<CA+TgmobN=+AoDE4JZioVpaRXJVCQ2Fa9Uw3TjC-OqnXWQ0uq1w@mail.gmail.com>
<CA+HiwqFOsJ21nmvoKtPkzxKfYzACajj-mAOoJ+-y_O6g+-v-aA@mail.gmail.com>
<CA+TgmoanT5K44bHZqMXxjAKwyF7nhajKBrw-p-+HSyEzOVqX1w@mail.gmail.com>
<CA+HiwqGbnNGttKr8fK2pAYcC1JAapo=1nXO7OQs2M7+HmoVVpw@mail.gmail.com>
<CA+HiwqEYCpEqh2LMDOp9mT+4-QoVe8HgFMKBjntEMCTZLpcCCA@mail.gmail.com>
<CA+TgmoYbpAC4fDWLBSg6MZ9SymyKrt3NSc42whin9Qbgm71nCQ@mail.gmail.com>
<[email protected]>
<CA+Tgmoakey5yO+TTp0Gntz5MdnMLhd+Ei-+HWuCszHJeVU_YVA@mail.gmail.com>
<CA+HiwqEDuz6OaM0Rv2D6kXgK0Cusr7HtquZTPjan+U76AJm0pg@mail.gmail.com>
<CA+HiwqH9-fAvpG-w9qYCcDWzK3vGPCMyw4f9nHzqkxXVuD1pxw@mail.gmail.com>
<CA+HiwqFq9UQiSJbGpJkmd0MkxqQLZWGk1+__tyk0QM+hmYHrrA@mail.gmail.com>
<CA+HiwqERQkaKkE8TwNmFP2SDBykttAqBkwh_Niif9FqST7F4Kg@mail.gmail.com>
<CA+HiwqGoORM+yfm8PhGWfHNUSBbSUgTQC=s5ieV8b4FX1yDBrg@mail.gmail.com>
<CAApHDvp_DjVVkgSV24+UF7p_yKWeepgoo+W2SWLLhNmjwHTVYQ@mail.gmail.com>
<CA+HiwqH7pYXfPZhHCKTmb_=C0z538wP8Mog+_j+vTi7JCVqnhQ@mail.gmail.com>
<CAApHDvrpWWtSRZXFO31H1KcC9csKuE31v2f7k6rVS9Yp5ibc7Q@mail.gmail.com>
<CA+HiwqEhn35nnFqv-0-T8mO1=ug7VcU2_3LnnToT2PzCNw9+4w@mail.gmail.com>
<CAApHDvqno9XyXr9aXyWOKQ96-fzdyh5dRZbbzTQopYNq1CNJCQ@mail.gmail.com>
<CA+HiwqEn-xV1B-QwxeTMs52MzGjaQFS24jiE-1LvMViHLNNfhg@mail.gmail.com>
<CA+HiwqEUEoAPX+UH0nM+pyA_-hXEH3oWdoBPEfD_uaR6GGu8nA@mail.gmail.com>
<CA+HiwqH4qQ_YVROr7TY0jSCuGn0oHhH79_DswOdXWN5UnMCBtQ@mail.gmail.com>
On Thu, 7 Apr 2022 at 20:28, Amit Langote <[email protected]> wrote:
> Here's an updated version. In Particular, I removed
> part_prune_results list from PortalData, in favor of anything that
> needs to look at the list can instead get it from the CachedPlan
> (PortalData.cplan). This makes things better in 2 ways:
Thanks for making those changes.
I'm not overly familiar with the data structures we use for planning
around plans between the planner and executor, but storing the pruning
results in CachedPlan seems pretty bad. I see you've stashed it in
there and invented a new memory context to stop leaks into the cache
memory.
Since I'm not overly familiar with these structures, I'm trying to
imagine why you made that choice and the best I can come up with was
that it was the most convenient thing you had to hand inside
CheckCachedPlan().
I don't really have any great ideas right now on how to make this
better. I wonder if GetCachedPlan() should be changed to return some
struct that wraps up the CachedPlan with some sort of executor prep
info struct that we can stash the list of PartitionPruneResults in,
and perhaps something else one day.
Some lesser important stuff that I think could be done better.
* Are you also able to put meaningful comments on the
PartitionPruneResult struct in execnodes.h?
* In create_append_plan() and create_merge_append_plan() you have the
same code to set the part_prune_index. Why not just move all that code
into make_partition_pruneinfo() and have make_partition_pruneinfo()
return the index and append to the PlannerInfo.partPruneInfos List?
* Why not forboth() here?
i = 0;
foreach(stmtlist_item, portal->stmts)
{
PlannedStmt *pstmt = lfirst_node(PlannedStmt, stmtlist_item);
PartitionPruneResult *part_prune_result = part_prune_results ?
list_nth(part_prune_results, i) :
NULL;
i++;
* It would be good if ReleaseExecutorLocks() already knew the RTIs
that were locked. Maybe the executor prep info struct I mentioned
above could also store the RTIs that have been locked already and
allow ReleaseExecutorLocks() to just iterate over those to release the
locks.
David
view thread (71+ messages) latest in thread
reply
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: [email protected]
Cc: [email protected], [email protected], [email protected], [email protected]
Subject: Re: generic plans and "initial" pruning
In-Reply-To: <CAApHDvroRU6wZC-Ruo5jQjEWOgcdey=gU0jwp-W2_S6DUVR8Bg@mail.gmail.com>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox