MIME-Version: 1.0
References: 
 <CA+HiwqFpZ80UJKr4tZus4Omgg7YESzFXKSwSHRW2Ap2=XSVyUA@mail.gmail.com>
 <54c35fb9-da3a-4754-ab8c-46ed0b612465@vondra.me>
 <CA+HiwqH8N-SxEB6SysEBsYNgV_KJs66k9Z2SNmqVzbBP-60yWg@mail.gmail.com>
 <684c70d7-180e-461d-9377-600c2db581ba@vondra.me>
 <CA+HiwqEmG9YCQvG6uux7sO=jKFSAW6hA4Ea-ymfD+JhJAe4PWQ@mail.gmail.com>
 <CA+HiwqE2FfJfH=siLiR3kJ13tmXZORAGTWsZc2r52o1_5BDv+g@mail.gmail.com>
 <bfcc1b78-aa39-4356-8f42-b8cfb9733d6e@vondra.me>
 <CA+HiwqFhkpXHAA=4NY5SqYXX08uq=nYtXcSByNZF=2MAy1UA7A@mail.gmail.com>
 <CA+HiwqHCcSoYfpMjFshaU1bj6NjreiDvMSDpVSeBmqk-kbWrPw@mail.gmail.com>
 <CA+HiwqHOejJk0_qMuM5g38h70hY_JvHMAKwnH3k=urfTXauPQA@mail.gmail.com>
 <CA+HiwqFsGKM82oaMby3VWYXf_XFpDAMeT+6SXgj-45HpTrS1dA@mail.gmail.com>
 <CA+HiwqFA5hUWYktt3VMh4zQOYMxqH-MpdX8eemfM+o-9dY-zbQ@mail.gmail.com>
 <CA+HiwqEn7bbUXaXO=SmUujBjJSHfS31cwQroHRBwT0sR=66bgg@mail.gmail.com>
 <CA+HiwqGGLDTd1ZTK1c0zv4La7XOVSVMqBuNtscJeh6FyUQvFvA@mail.gmail.com>
 <CA+HiwqE2JFiqqrXdyJVQWY-fMGwzDkLqjXQdUKbPaCpDpxd_2g@mail.gmail.com>
 <CA+HiwqFp3jZGSz==QjeuV62_62F6+V6b62=Uqvy99sW_gsgWBA@mail.gmail.com>
 <CAHewXNkUz9XGG8nnoxZaw35e+5bQVVP=eeJE4cW4V2e+P9ndFw@mail.gmail.com>
 <CA+HiwqFKSpfYruzcVz-5CcFxg7gMa+ycXjMa2aPz_J_P4LGXTg@mail.gmail.com>
 <605328.1747710381@sss.pgh.pa.us>
 <CA+HiwqEQ1oME-hcDXwC9rGQb=u7MdUFG3Sc=Qg27uH480v10FA@mail.gmail.com>
 <691261.1747755511@sss.pgh.pa.us>
 <CA+HiwqGXMLSQyJvynWF40yNwBAx-pXtxemReP8L+C+kaUa5v5A@mail.gmail.com>
 <CA+HiwqGBfMgcxokEH_mg6s=ttLFm54dj4hT6yXydU2t0g6oQ3g@mail.gmail.com>
 <CA+HiwqEEkGfMc_LSJhfz96o-czVS4B59Vhw6i1_t58ZGqhP8VA@mail.gmail.com>
 <CA+HiwqHAd+9nptjxP6=KrcKA1BMsS6pbB3B2oaojwdyH_wBWCA@mail.gmail.com>
 <CA+HiwqE7_YpU--EsrhvNqcZ+10+92EGFaX5609AUJb9ENLntnQ@mail.gmail.com>
 <CA+HiwqEF9SgKyQ1HrYOURpv8DGRGHDNwBT9Y6yEBVCW+=kh_=w@mail.gmail.com>
 <CA+HiwqFpEHBjosRackQhm6yKKnHgqm8Ewkn=qsctT1N0PqVSrg@mail.gmail.com>
 <CA+HiwqGJP91Qed0EjuB72Lv4_QAiVOMYjya7GA0aas8K6NZUZA@mail.gmail.com>
 <A93C2AC9-8B93-45AB-B56C-E6771D988808@gmail.com>
 <CA+HiwqE7LDSoaF024Mt9v1Gt-uE-WoT9GawC5ds45SaPczV8Qw@mail.gmail.com>
 <CA+HiwqGn38DsKgMYKWZ6jyv3_oqCSB0j+XucTjNM0S+BFsQpVA@mail.gmail.com>
 <CA+HiwqGFNe7kBkKZm0KtG_CFfw-ciK659SJMGP0CWVaa2q8rmw@mail.gmail.com>
 <CA+HiwqELAcgVg_3Gb4VTOpC6wcNhHP0m-8OJFG0MeGRo0M=_4Q@mail.gmail.com>
 <CA+HiwqHBxDL=3qQa1f-sBOBZqB88EiVAiagXF3X8Kagpr6Yhpw@mail.gmail.com>
 <CA+HiwqFx0kmGqSDcLrE37KkHS2T9O1NoBitZT4mA4yJBBt_QjA@mail.gmail.com>
 <CA+HiwqGq=xQvE0oCeOX_oXWq2iyNs5q9UwopyQ2uXF2kJPXTDg@mail.gmail.com>
In-Reply-To: 
 <CA+HiwqGq=xQvE0oCeOX_oXWq2iyNs5q9UwopyQ2uXF2kJPXTDg@mail.gmail.com>
From: Amit Langote <amitlangote09@gmail.com>
Date: Fri, 27 Mar 2026 18:00:20 +0900
Message-ID: 
 <CA+HiwqHN9x7ufTz3EfAA3-Zq3NOTeZMKtBatmevMesybwBUaAw@mail.gmail.com>
Subject: Re: generic plans and "initial" pruning
To: Chao Li <li.evan.chao@gmail.com>
Cc: Tom Lane <tgl@sss.pgh.pa.us>, Tender Wang <tndrwang@gmail.com>,
	Alexander Lakhin <exclusion@gmail.com>, Tomas Vondra <tomas@vondra.me>,
	Robert Haas <robertmhaas@gmail.com>,
 Alvaro Herrera <alvherre@alvh.no-ip.org>,
	Andres Freund <andres@anarazel.de>, Daniel Gustafsson <daniel@yesql.se>,
	David Rowley <dgrowleyml@gmail.com>,
 PostgreSQL Hackers <pgsql-hackers@postgresql.org>,
	Thom Brown <thom@linux.com>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Archived-At: 
 <https://www.postgresql.org/message-id/CA%2BHiwqHN9x7ufTz3EfAA3-Zq3NOTeZMKtBatmevMesybwBUaAw%40mail.gmail.com>
Precedence: bulk

On Thu, Mar 26, 2026 at 6:24=E2=80=AFPM Amit Langote <amitlangote09@gmail.c=
om> wrote:
> On Wed, Mar 25, 2026 at 4:39=E2=80=AFPM Amit Langote <amitlangote09@gmail=
.com> wrote:
> > On Fri, Mar 20, 2026 at 2:20=E2=80=AFAM Amit Langote <amitlangote09@gma=
il.com> wrote:
> > > On Mon, Mar 9, 2026 at 1:41=E2=80=AFPM Amit Langote <amitlangote09@gm=
ail.com> wrote:
> > > Stepping back -- the core question is whether running executor logic
> > > (pruning) inside GetCachedPlan() is acceptable at all. The plan cache
> > > and executor have always had a clean boundary: plan cache locks
> > > everything, executor runs. This optimization necessarily crosses that
> > > line, because the information needed to decide which locks to skip
> > > (pruning results) can only come from executor machinery.
> > >
> > > The proposed approach has GetCachedPlan() call ExecutorPrep() to do a
> > > limited subset of executor work (range table init, permissions,
> > > pruning), carry the results out through CachedPlanPrepData, and leave
> > > the CachedPlan itself untouched. The executor already has a multi-ste=
p
> > > protocol: start/run/end. prep/start/run/end is just a finer
> > > decomposition of what InitPlan() was already doing inside
> > > ExecutorStart().
> > >
> > > Of the attached patches, I'm targeting 0001-0003 for commit. 0004 (SQ=
L
> > > function support) and 0005 (parallel worker reuse) are useful
> > > follow-ons but not essential.  The optimization works without them fo=
r
> > > most cases, and they can be reviewed and committed separately.
> > >
> > > If there's a cleaner way to avoid locking pruned partitions without
> > > the plumbing this patch adds, I haven't found it in the year since th=
e
> > > revert.  I'd welcome a pointer if you see one.  Failing that, I think
> > > this is the right trade-off, but it's a judgment call about where to
> > > hold your nose.
> > >
> > > Tom, I'd value your opinion on whether this approach is something
> > > you'd be comfortable seeing in the tree.
> >
> > Attached is an updated set with some cleanup after another pass.
> >
> > - Removed ExecCreatePartitionPruneStates() from 0001. In 0001-0003,
> > ExecDoInitialPruning() handles both setup and pruning internally; the
> > split isn't needed yet.
> >
> > - Tightened commit messages to describe what each commit does now, not
> > what later commits will use it for. In particular, 0002 is upfront
> > that the portal/SPI/EXPLAIN plumbing is scaffolding that 0003 lights
> > up.
> >
> > - Updated setrefs.c comment for firstResultRels to drop a blanket
> > claim about one ModifyTable per query level.
> >
> > As before, 0001-0003 is the focus, maybe 0004 which teaches the new
> > GetCachedPlan() pruning-aware contract to its relatively new user in
> > function.c.
>
> While reviewing the patch more carefully, I realized there's a
> correctness issue when rule rewriting causes a single statement to
> expand into multiple PlannedStmts in one CachedPlan.
>
> PortalRunMulti() executes those statements sequentially, with
> CommandCounterIncrement() between them, so Q2's ExecutorStart()
> normally sees the effects of Q1.
>
> With the patch, though, AcquireExecutorLocksUnpruned() runs
> ExecutorPrep() on all PlannedStmts in one pass during GetCachedPlan(),
> before any statement executes. If a later statement has
> initial-pruning expressions that read data modified by an earlier one,
> pruning can see stale results.
>
> There's also a memory lifetime issue: PortalRunMulti() calls
> MemoryContextDeleteChildren(portalContext) between statements, which
> destroys EStates prepared for later statements.
>
> Here's a concrete case demonstrating the semantic issue:
>
>   create table multistmt_pt (a int, b int) partition by list (a);
>   create table multistmt_pt_1 partition of multistmt_pt for values in (1)=
;
>   create table multistmt_pt_2 partition of multistmt_pt for values in (2)=
;
>   insert into multistmt_pt values (1, 0), (2, 0);
>
>   create table prune_config (val int);
>   insert into prune_config values (1);
>
>   create function get_prune_val() returns int as $$
>     select val from prune_config;
>   $$ language sql stable;
>
>   -- rule action runs first, updating prune_config before the
>   -- original statement's pruning would normally be evaluated
>   create rule config_upd_rule as on update to multistmt_pt
>     do also update prune_config set val =3D 2;
>
>   set plan_cache_mode to force_generic_plan;
>   prepare multi_q as
>     update multistmt_pt set b =3D b + 1 where a =3D get_prune_val();
>   execute multi_q;  -- creates the generic plan
>
>   -- reset for the real test
>   update prune_config set val =3D 1;
>   update multistmt_pt set b =3D 0;
>
>   -- second execute reuses the plan
>   execute multi_q;
>   select * from multistmt_pt order by a;
>
> Without the patch: the rule action updates prune_config to val=3D2
> first, then after CCI the original statement's initial pruning calls
> get_prune_val(), gets 2, prunes to multistmt_pt_2, and updates it
> correctly: (1, 0), (2, 1).
>
> With the patch as it stood: both statements' pruning runs during
> GetCachedPlan() before either executes. The original statement's
> pruning sees val=3D1, prunes to multistmt_pt_1, and multistmt_pt_2 is
> never touched.
>
> The fix is to skip pruning-aware locking for CachedPlans containing
> multiple PlannedStmts, falling back to locking all partitions.
> Single-statement plans are unchanged.

For good measure, I also verified that Tom's test case from last May
[1] that prompted the revert of the previous commit works correctly
with this patch. When the DO ALSO rule is created mid-execution, the
plan gets invalidated and rebuilt as a multi-statement CachedPlan,
which triggers the fallback to locking all partitions. No assertions,
no crashes.

--=20
Thanks, Amit Langote

[1] https://postgr.es/m/605328.1747710381@sss.pgh.pa.us