On Thu, Dec 5, 2024 at 8:09 PM Tom Lane <tgl@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
