public inbox for [email protected]
help / color / mirror / Atom feedFrom: Tom Lane <[email protected]>
To: Amit Langote <[email protected]>
Cc: Tender Wang <[email protected]>
Cc: Alexander Lakhin <[email protected]>
Cc: Tomas Vondra <[email protected]>
Cc: Robert Haas <[email protected]>
Cc: Alvaro Herrera <[email protected]>
Cc: Andres Freund <[email protected]>
Cc: Daniel Gustafsson <[email protected]>
Cc: David Rowley <[email protected]>
Cc: PostgreSQL Hackers <[email protected]>
Cc: Thom Brown <[email protected]>
Subject: Re: generic plans and "initial" pruning
Date: Tue, 20 May 2025 11:38:31 -0400
Message-ID: <[email protected]> (raw)
In-Reply-To: <CA+HiwqEQ1oME-hcDXwC9rGQb=u7MdUFG3Sc=Qg27uH480v10FA@mail.gmail.com>
References: <CA+HiwqFpZ80UJKr4tZus4Omgg7YESzFXKSwSHRW2Ap2=XSVyUA@mail.gmail.com>
<[email protected]>
<CA+HiwqH8N-SxEB6SysEBsYNgV_KJs66k9Z2SNmqVzbBP-60yWg@mail.gmail.com>
<[email protected]>
<CA+HiwqEmG9YCQvG6uux7sO=jKFSAW6hA4Ea-ymfD+JhJAe4PWQ@mail.gmail.com>
<CA+HiwqE2FfJfH=siLiR3kJ13tmXZORAGTWsZc2r52o1_5BDv+g@mail.gmail.com>
<[email protected]>
<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>
<[email protected]>
<CA+HiwqEQ1oME-hcDXwC9rGQb=u7MdUFG3Sc=Qg27uH480v10FA@mail.gmail.com>
Amit Langote <[email protected]> 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
view thread (114+ 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], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected]
Subject: Re: generic plans and "initial" pruning
In-Reply-To: <[email protected]>
* 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