From: Tom Lane <tgl@sss.pgh.pa.us>
To: Amit Langote <amitlangote09@gmail.com>
cc: 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>
Subject: Re: generic plans and "initial" pruning
In-reply-to: 
 <CA+HiwqEQ1oME-hcDXwC9rGQb=u7MdUFG3Sc=Qg27uH480v10FA@mail.gmail.com>
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> <C!
 AHewXNnEwVQoXBiR+nC09R20Psr2xXjOuhatC-kHfr0-B2pcVw@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>
Comments: In-reply-to Amit Langote <amitlangote09@gmail.com>
	message dated "Tue, 20 May 2025 22:25:41 +0900"
MIME-Version: 1.0
Content-Type: text/plain; charset="UTF-8"
Content-ID: <691260.1747755511.1@sss.pgh.pa.us>
Content-Transfer-Encoding: 8bit
Date: Tue, 20 May 2025 11:38:31 -0400
Message-ID: <691261.1747755511@sss.pgh.pa.us>
Archived-At: 
 <https://www.postgresql.org/message-id/691261.1747755511%40sss.pgh.pa.us>
Precedence: bulk

Amit Langote <amitlangote09@gmail.com> writes:
> Thanks for pointing out the hole in the current handling of
> CachedPlan->stmt_list. You're right that the approach of preserving
> the list structure while replacing its contents in-place doesn’t hold
> up when the rewriter adds or removes statements dynamically. There
> might be other cases that neither of us have tried.  I don’t think
> that mechanism is salvageable.

> To address the issue without needing a full revert, I’m considering
> dropping UpdateCachedPlan() and removing the associated MemoryContext
> dance to preserve CachedPlan->stmt_list structure. Instead, the
> executor would replan the necessary query into a transient list of
> PlannedStmts, leaving the original CachedPlan untouched. That avoids
> mutating shared plan state during execution and still enables deferred
> locking in the vast majority of cases.

Yeah, I think messing with the CachedPlan is just fundamentally wrong.
It breaks the invariant that the executor should not scribble on what
it's handed --- maybe not as obviously as some other cases, but it's
still not a good design.

I kind of feel that we ought to take two steps back and think
about what it even means to have a generic plan in this situation.
Perhaps we should simply refuse to use that code path if there are
prunable partitioned tables involved?

> Let me know what you think -- I’ll hold off on posting a revert or a
> replacement until we’ve agreed on the path forward.

I had not looked at 525392d57 in any detail before (the claim in
the commit message that I reviewed it is a figment of someone's
imagination).  Now that I have, I'm still going to argue for revert.
Aside from the points above, I really hate what's been done to the
fundamental executor APIs.  The fact that ExecutorStart callers have
to know about this is as ugly as can be.  I also don't like the
fact that it's added overhead in cases where there can be no benefit
(notice that my test case doesn't even involve a partitioned table).

I still like the core idea of deferring locking, but I don't like
anything about this implementation of it.  It seems like there has
to be a better and simpler way.

			regards, tom lane