Change WorkTableScan to not support backward scan. The apparent support
authorTom Lane <tgl@sss.pgh.pa.us>
Tue, 28 Oct 2008 17:13:51 +0000 (17:13 +0000)
committerTom Lane <tgl@sss.pgh.pa.us>
Tue, 28 Oct 2008 17:13:51 +0000 (17:13 +0000)
didn't actually work, because nodeRecursiveunion.c creates the underlying
tuplestore with backward scan disabled; which is a decision that we shouldn't
reverse because of performance cost.  We could imagine adding signaling from
WorkTableScan to RecursiveUnion about whether backward scan is needed ...
but in practice it'd be a waste of effort, because there simply isn't any
current or plausible future scenario where WorkTableScan would be called on
to scan backward.  So just dike out the code that claims to support it.

src/backend/executor/execAmi.c
src/backend/executor/nodeWorktablescan.c

index 485a969d4602c64e989af623d72af99e7caf91bb..b2c4e85e50994b76ed082f6c85ee5891ee7dc1c5 100644 (file)
@@ -419,7 +419,6 @@ ExecSupportsBackwardScan(Plan *node)
                case T_FunctionScan:
                case T_ValuesScan:
                case T_CteScan:
-               case T_WorkTableScan:
                        return TargetListSupportsBackwardScan(node->targetlist);
 
                case T_IndexScan:
index 4bed9ee0e34bfb260ecb974e7e675acbdb16ce0c..4150d43aa535fc3e567695af7f9c5c6d6e10879e 100644 (file)
@@ -31,14 +31,21 @@ WorkTableScanNext(WorkTableScanState *node)
 {
        TupleTableSlot *slot;
        EState     *estate;
-       ScanDirection direction;
        Tuplestorestate *tuplestorestate;
 
        /*
         * get information from the estate and scan state
+        *
+        * Note: we intentionally do not support backward scan.  Although it would
+        * take only a couple more lines here, it would force nodeRecursiveunion.c
+        * to create the tuplestore with backward scan enabled, which has a
+        * performance cost.  In practice backward scan is never useful for a
+        * worktable plan node, since it cannot appear high enough in the plan
+        * tree of a scrollable cursor to be exposed to a backward-scan
+        * requirement.  So it's not worth expending effort to support it.
         */
        estate = node->ss.ps.state;
-       direction = estate->es_direction;
+       Assert(ScanDirectionIsForward(estate->es_direction));
 
        tuplestorestate = node->rustate->working_table;
 
@@ -46,9 +53,7 @@ WorkTableScanNext(WorkTableScanState *node)
         * Get the next tuple from tuplestore. Return NULL if no more tuples.
         */
        slot = node->ss.ss_ScanTupleSlot;
-       (void) tuplestore_gettupleslot(tuplestorestate,
-                                                                  ScanDirectionIsForward(direction),
-                                                                  slot);
+       (void) tuplestore_gettupleslot(tuplestorestate, true, slot);
        return slot;
 }
 
@@ -114,7 +119,7 @@ ExecInitWorkTableScan(WorkTableScan *node, EState *estate, int eflags)
        WorkTableScanState *scanstate;
 
        /* check for unsupported flags */
-       Assert(!(eflags & EXEC_FLAG_MARK));
+       Assert(!(eflags & (EXEC_FLAG_BACKWARD | EXEC_FLAG_MARK)));
 
        /*
         * WorkTableScan should not have any children.