On Tue, Aug 20, 2024 at 3:21 AM Robert Haas <robertmh...@gmail.com> wrote: > On Mon, Aug 19, 2024 at 1:52 PM Tom Lane <t...@sss.pgh.pa.us> wrote: > > Robert Haas <robertmh...@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
