On Tue, Aug 20, 2024 at 3:21 AM Robert Haas <robertmhaas@gmail.com> wrote:
> On Mon, Aug 19, 2024 at 1:52 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
> > Robert Haas <robertmhaas@gmail.com> writes:
> > > But that seems somewhat incidental to what this thread is about.
> >
> > Perhaps. But if we're running into issues related to that, it might
> > be good to set aside the long-term goal for a bit and come up with
> > a cleaner answer for intra-session locking. That could allow the
> > pruning problem to be solved more cleanly in turn, and it'd be
> > an improvement even if not.
>
> Maybe, but the pieces aren't quite coming together for me. Solving
> this would mean that if we execute a stale plan, we'd be more likely
> to get a good error and less likely to get a bad, nasty-looking
> internal error, or a crash. That's good on its own terms, but we don't
> really want user queries to produce errors at all, so I don't think
> we'd feel any more free to rearrange the order of operations than we
> do today.
Yeah, it's unclear whether executing a potentially stale plan is an
acceptable tradeoff compared to replanning, especially if it occurs
rarely. Personally, I would prefer that it is.
> > > Do you have a view on what the way forward might be?
> >
> > I'm fresh out of ideas at the moment, other than having a hope that
> > divide-and-conquer (ie, solving subproblems first) might pay off.
>
> Fair enough, but why do you think that the original approach of
> creating a data structure from within the plan cache mechanism
> (probably via a call into some new executor entrypoint) and then
> feeding that through to ExecutorRun() time can't work?
That would be ExecutorStart(). The data structure need not be
referenced after ExecInitNode().
> Is it possible
> you latched onto some non-optimal decisions that the early versions of
> the patch made, rather than there being a fundamental problem with the
> concept?
>
> I actually thought the do-it-at-executorstart-time approach sounded
> pretty good, even though we might have to abandon planstate tree
> initialization partway through, right up until Amit started talking
> about moving ExecutorStart() from PortalRun() to PortalStart(), which
> I have a feeling is going to create a bigger problem than we can
> solve. I think if we want to save that approach, we should try to
> figure out if we can teach the plancache to replan one query from a
> list without replanning the others, which seems like it might allow us
> to keep the order of major operations unchanged. Otherwise, it makes
> sense to me to have another go at the other approach, at least to make
> sure we understand clearly why it can't work.
+1
--
Thanks, Amit Langote
