On Thu, Dec 5, 2024 at 8:09 PM Tom Lane <t...@sss.pgh.pa.us> wrote: > After looking at this further, I think this whole "run_once" > business is badly designed, worse documented, and redundant > (as well as buggy). It can be replaced with three self-contained > lines in ExecutePlan, as per the attached.
Did you look at the commit message for 691b8d59281b5177f16fe80858df921f77a8e955? IMHO, that's pretty clear about what the goals of this were. I didn't consider the scenario mentioned by Nagata-san in his original post, namely, executing a portal with a row count of 0 and then doing the same thing again. So in that sense the assertion fulfilled its intended purpose: alerting somebody that there is a case which is reachable but which the original programmer believed to be unreachable. I haven't tested, but I'm guessing that what happens in Nagata-san's case with the committed fix is that we execute the plan once to completion in parallel mode; and then the second execute message executes the same plan tree again in non-parallel mode but this time it ends up returning zero tuples because it's already been executed to completion. If that is correct, then I think this is subtly changing the rules for parallel Plan trees: before, the rule as I understood it, was "only complete execution must supported" but now it's "complete executed must be supported and, in addition, a no-op re-execution after complete execution must be supported". Off the top of my head that seems like it should be fine. -- Robert Haas EDB: http://www.enterprisedb.com
