Doc: add some commentary about ExecutorRun's NoMovement special case.
authorTom Lane <[email protected]>
Tue, 10 Dec 2024 22:17:28 +0000 (17:17 -0500)
committerTom Lane <[email protected]>
Tue, 10 Dec 2024 22:17:28 +0000 (17:17 -0500)
Robert Haas expressed concern about whether commit 3eea7a0c9 exposed
the parallel-execution machinery to a case it isn't tested for, namely
a second non-parallel execution of a plan after a parallel execution.
Investigation shows that that can't happen because of pquery.c's
manipulation of the scan direction, but it sure wasn't obvious to
start with.  Add some commentary about that.

Discussion: https://postgr.es/m/CA+TgmoagyKQy=HFw+wLo0AKTYWHui+iKswZ8Jnqqd-cFby-WVg@mail.gmail.com

src/backend/executor/execMain.c

index 13f3fcdaee93f6f750d22b28c5f042cf9a5a7880..1c12d6ebff04dee4a6d7248c7927b84ac384800c 100644 (file)
@@ -348,7 +348,16 @@ standard_ExecutorRun(QueryDesc *queryDesc,
        dest->rStartup(dest, operation, queryDesc->tupDesc);
 
    /*
-    * run plan
+    * Run plan, unless direction is NoMovement.
+    *
+    * Note: pquery.c selects NoMovement if a prior call already reached
+    * end-of-data in the user-specified fetch direction.  This is important
+    * because various parts of the executor can misbehave if called again
+    * after reporting EOF.  For example, heapam.c would actually restart a
+    * heapscan and return all its data afresh.  There is also some doubt
+    * about whether a parallel plan would operate properly if an additional,
+    * necessarily non-parallel execution request occurs after completing a
+    * parallel execution.  (That case should work, but it's untested.)
     */
    if (!ScanDirectionIsNoMovement(direction))
        ExecutePlan(queryDesc,