From: Tom Lane Date: Fri, 21 Nov 2014 01:20:54 +0000 (-0500) Subject: Remove dead code supporting mark/restore in SeqScan, TidScan, ValuesScan. X-Git-Tag: REL9_5_ALPHA1~1173 X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=adbfab119b308a7e0e6b1305de9be222cfd5c85b;p=postgresql Remove dead code supporting mark/restore in SeqScan, TidScan, ValuesScan. There seems no prospect that any of this will ever be useful, and indeed it's questionable whether some of it would work if it ever got called; it's certainly not been exercised in a very long time, if ever. So let's get rid of it, and make the comments about mark/restore in execAmi.c less wishy-washy. The mark/restore support for Result nodes is also currently dead code, but that's due to planner limitations not because it's impossible that it could be useful. So I left it in. --- diff --git a/src/backend/access/heap/heapam.c b/src/backend/access/heap/heapam.c index c6e1eb79b2..df4853bad1 100644 --- a/src/backend/access/heap/heapam.c +++ b/src/backend/access/heap/heapam.c @@ -27,8 +27,6 @@ * heap_multi_insert - insert multiple tuples into a relation * heap_delete - delete a tuple from a relation * heap_update - replace a tuple in a relation with another tuple - * heap_markpos - mark scan position - * heap_restrpos - restore position to marked location * heap_sync - sync heap, for when no WAL has been written * * NOTES @@ -280,9 +278,6 @@ initscan(HeapScanDesc scan, ScanKey key, bool is_rescan) scan->rs_cbuf = InvalidBuffer; scan->rs_cblock = InvalidBlockNumber; - /* we don't have a marked position... */ - ItemPointerSetInvalid(&(scan->rs_mctid)); - /* page-at-a-time fields are always invalid when not rs_inited */ /* @@ -6317,71 +6312,6 @@ heap_tuple_needs_freeze(HeapTupleHeader tuple, TransactionId cutoff_xid, return false; } -/* ---------------- - * heap_markpos - mark scan position - * ---------------- - */ -void -heap_markpos(HeapScanDesc scan) -{ - /* Note: no locking manipulations needed */ - - if (scan->rs_ctup.t_data != NULL) - { - scan->rs_mctid = scan->rs_ctup.t_self; - if (scan->rs_pageatatime) - scan->rs_mindex = scan->rs_cindex; - } - else - ItemPointerSetInvalid(&scan->rs_mctid); -} - -/* ---------------- - * heap_restrpos - restore position to marked location - * ---------------- - */ -void -heap_restrpos(HeapScanDesc scan) -{ - /* XXX no amrestrpos checking that ammarkpos called */ - - if (!ItemPointerIsValid(&scan->rs_mctid)) - { - scan->rs_ctup.t_data = NULL; - - /* - * unpin scan buffers - */ - if (BufferIsValid(scan->rs_cbuf)) - ReleaseBuffer(scan->rs_cbuf); - scan->rs_cbuf = InvalidBuffer; - scan->rs_cblock = InvalidBlockNumber; - scan->rs_inited = false; - } - else - { - /* - * If we reached end of scan, rs_inited will now be false. We must - * reset it to true to keep heapgettup from doing the wrong thing. - */ - scan->rs_inited = true; - scan->rs_ctup.t_self = scan->rs_mctid; - if (scan->rs_pageatatime) - { - scan->rs_cindex = scan->rs_mindex; - heapgettup_pagemode(scan, - NoMovementScanDirection, - 0, /* needn't recheck scan keys */ - NULL); - } - else - heapgettup(scan, - NoMovementScanDirection, - 0, /* needn't recheck scan keys */ - NULL); - } -} - /* * If 'tuple' contains any visible XID greater than latestRemovedXid, * ratchet forwards latestRemovedXid to the greatest one found. diff --git a/src/backend/executor/execAmi.c b/src/backend/executor/execAmi.c index 643aaace3a..7027d7f805 100644 --- a/src/backend/executor/execAmi.c +++ b/src/backend/executor/execAmi.c @@ -271,16 +271,20 @@ ExecReScan(PlanState *node) * ExecMarkPos * * Marks the current scan position. + * + * NOTE: mark/restore capability is currently needed only for plan nodes + * that are the immediate inner child of a MergeJoin node. Since MergeJoin + * requires sorted input, there is never any need to support mark/restore in + * node types that cannot produce sorted output. There are some cases in + * which a node can pass through sorted data from its child; if we don't + * implement mark/restore for such a node type, the planner compensates by + * inserting a Material node above that node. */ void ExecMarkPos(PlanState *node) { switch (nodeTag(node)) { - case T_SeqScanState: - ExecSeqMarkPos((SeqScanState *) node); - break; - case T_IndexScanState: ExecIndexMarkPos((IndexScanState *) node); break; @@ -289,14 +293,6 @@ ExecMarkPos(PlanState *node) ExecIndexOnlyMarkPos((IndexOnlyScanState *) node); break; - case T_TidScanState: - ExecTidMarkPos((TidScanState *) node); - break; - - case T_ValuesScanState: - ExecValuesMarkPos((ValuesScanState *) node); - break; - case T_CustomScanState: ExecCustomMarkPos((CustomScanState *) node); break; @@ -338,10 +334,6 @@ ExecRestrPos(PlanState *node) { switch (nodeTag(node)) { - case T_SeqScanState: - ExecSeqRestrPos((SeqScanState *) node); - break; - case T_IndexScanState: ExecIndexRestrPos((IndexScanState *) node); break; @@ -350,14 +342,6 @@ ExecRestrPos(PlanState *node) ExecIndexOnlyRestrPos((IndexOnlyScanState *) node); break; - case T_TidScanState: - ExecTidRestrPos((TidScanState *) node); - break; - - case T_ValuesScanState: - ExecValuesRestrPos((ValuesScanState *) node); - break; - case T_CustomScanState: ExecCustomRestrPos((CustomScanState *) node); break; @@ -386,14 +370,6 @@ ExecRestrPos(PlanState *node) * This is used during planning and so must accept a Path, not a Plan. * We keep it here to be adjacent to the routines above, which also must * know which plan types support mark/restore. - * - * XXX Ideally, all plan node types would support mark/restore, and this - * wouldn't be needed. For now, this had better match the routines above. - * - * (However, since the only present use of mark/restore is in mergejoin, - * there is no need to support mark/restore in any plan type that is not - * capable of generating ordered output. So the seqscan, tidscan, - * and valuesscan support is actually useless code at present.) */ bool ExecSupportsMarkRestore(Path *pathnode) @@ -405,11 +381,8 @@ ExecSupportsMarkRestore(Path *pathnode) */ switch (pathnode->pathtype) { - case T_SeqScan: case T_IndexScan: case T_IndexOnlyScan: - case T_TidScan: - case T_ValuesScan: case T_Material: case T_Sort: return true; @@ -426,7 +399,11 @@ ExecSupportsMarkRestore(Path *pathnode) * Although Result supports mark/restore if it has a child plan * that does, we presently come here only for ResultPath nodes, * which represent Result plans without a child plan. So there is - * nothing to recurse to and we can just say "false". + * nothing to recurse to and we can just say "false". (This means + * that Result's support for mark/restore is in fact dead code. + * We keep it since it's not much code, and someday the planner + * might be smart enough to use it. That would require making + * this function smarter too, of course.) */ Assert(IsA(pathnode, ResultPath)); return false; diff --git a/src/backend/executor/nodeSeqscan.c b/src/backend/executor/nodeSeqscan.c index ab13e4729e..53cfda5f9c 100644 --- a/src/backend/executor/nodeSeqscan.c +++ b/src/backend/executor/nodeSeqscan.c @@ -19,8 +19,6 @@ * ExecInitSeqScan creates and initializes a seqscan node. * ExecEndSeqScan releases any storage allocated. * ExecReScanSeqScan rescans the relation - * ExecSeqMarkPos marks scan position - * ExecSeqRestrPos restores scan position */ #include "postgres.h" @@ -274,39 +272,3 @@ ExecReScanSeqScan(SeqScanState *node) ExecScanReScan((ScanState *) node); } - -/* ---------------------------------------------------------------- - * ExecSeqMarkPos(node) - * - * Marks scan position. - * ---------------------------------------------------------------- - */ -void -ExecSeqMarkPos(SeqScanState *node) -{ - HeapScanDesc scan = node->ss_currentScanDesc; - - heap_markpos(scan); -} - -/* ---------------------------------------------------------------- - * ExecSeqRestrPos - * - * Restores scan position. - * ---------------------------------------------------------------- - */ -void -ExecSeqRestrPos(SeqScanState *node) -{ - HeapScanDesc scan = node->ss_currentScanDesc; - - /* - * Clear any reference to the previously returned tuple. This is needed - * because the slot is simply pointing at scan->rs_cbuf, which - * heap_restrpos will change; we'd have an internally inconsistent slot if - * we didn't do this. - */ - ExecClearTuple(node->ss_ScanTupleSlot); - - heap_restrpos(scan); -} diff --git a/src/backend/executor/nodeTidscan.c b/src/backend/executor/nodeTidscan.c index 3560f8440a..1b2454d49d 100644 --- a/src/backend/executor/nodeTidscan.c +++ b/src/backend/executor/nodeTidscan.c @@ -19,8 +19,6 @@ * ExecInitTidScan creates and initializes state info. * ExecReScanTidScan rescans the tid relation. * ExecEndTidScan releases all storage. - * ExecTidMarkPos marks scan position. - * ExecTidRestrPos restores scan position. */ #include "postgres.h" @@ -440,34 +438,6 @@ ExecEndTidScan(TidScanState *node) ExecCloseScanRelation(node->ss.ss_currentRelation); } -/* ---------------------------------------------------------------- - * ExecTidMarkPos - * - * Marks scan position by marking the current tid. - * Returns nothing. - * ---------------------------------------------------------------- - */ -void -ExecTidMarkPos(TidScanState *node) -{ - node->tss_MarkTidPtr = node->tss_TidPtr; -} - -/* ---------------------------------------------------------------- - * ExecTidRestrPos - * - * Restores scan position by restoring the current tid. - * Returns nothing. - * - * XXX Assumes previously marked scan position belongs to current tid - * ---------------------------------------------------------------- - */ -void -ExecTidRestrPos(TidScanState *node) -{ - node->tss_TidPtr = node->tss_MarkTidPtr; -} - /* ---------------------------------------------------------------- * ExecInitTidScan * diff --git a/src/backend/executor/nodeValuesscan.c b/src/backend/executor/nodeValuesscan.c index d49a473386..8365b8c99f 100644 --- a/src/backend/executor/nodeValuesscan.c +++ b/src/backend/executor/nodeValuesscan.c @@ -246,7 +246,6 @@ ExecInitValuesScan(ValuesScan *node, EState *estate, int eflags) /* * Other node-specific setup */ - scanstate->marked_idx = -1; scanstate->curr_idx = -1; scanstate->array_len = list_length(node->values_lists); @@ -293,30 +292,6 @@ ExecEndValuesScan(ValuesScanState *node) ExecClearTuple(node->ss.ss_ScanTupleSlot); } -/* ---------------------------------------------------------------- - * ExecValuesMarkPos - * - * Marks scan position. - * ---------------------------------------------------------------- - */ -void -ExecValuesMarkPos(ValuesScanState *node) -{ - node->marked_idx = node->curr_idx; -} - -/* ---------------------------------------------------------------- - * ExecValuesRestrPos - * - * Restores scan position. - * ---------------------------------------------------------------- - */ -void -ExecValuesRestrPos(ValuesScanState *node) -{ - node->curr_idx = node->marked_idx; -} - /* ---------------------------------------------------------------- * ExecReScanValuesScan * diff --git a/src/include/access/heapam.h b/src/include/access/heapam.h index 9cd66a1b0f..f43b4829e9 100644 --- a/src/include/access/heapam.h +++ b/src/include/access/heapam.h @@ -161,9 +161,6 @@ extern void simple_heap_delete(Relation relation, ItemPointer tid); extern void simple_heap_update(Relation relation, ItemPointer otid, HeapTuple tup); -extern void heap_markpos(HeapScanDesc scan); -extern void heap_restrpos(HeapScanDesc scan); - extern void heap_sync(Relation relation); /* in heap/pruneheap.c */ diff --git a/src/include/access/relscan.h b/src/include/access/relscan.h index 8beb1be882..f2c7ca1673 100644 --- a/src/include/access/relscan.h +++ b/src/include/access/relscan.h @@ -48,11 +48,9 @@ typedef struct HeapScanDescData BlockNumber rs_cblock; /* current block # in scan, if any */ Buffer rs_cbuf; /* current buffer in scan, if any */ /* NB: if rs_cbuf is not InvalidBuffer, we hold a pin on that buffer */ - ItemPointerData rs_mctid; /* marked scan position, if any */ /* these fields only used in page-at-a-time mode and for bitmap scans */ int rs_cindex; /* current tuple's index in vistuples */ - int rs_mindex; /* marked tuple's saved index */ int rs_ntuples; /* number of visible tuples on page */ OffsetNumber rs_vistuples[MaxHeapTuplesPerPage]; /* their offsets */ } HeapScanDescData; diff --git a/src/include/executor/nodeSeqscan.h b/src/include/executor/nodeSeqscan.h index 86f99d8384..4dc74314be 100644 --- a/src/include/executor/nodeSeqscan.h +++ b/src/include/executor/nodeSeqscan.h @@ -19,8 +19,6 @@ extern SeqScanState *ExecInitSeqScan(SeqScan *node, EState *estate, int eflags); extern TupleTableSlot *ExecSeqScan(SeqScanState *node); extern void ExecEndSeqScan(SeqScanState *node); -extern void ExecSeqMarkPos(SeqScanState *node); -extern void ExecSeqRestrPos(SeqScanState *node); extern void ExecReScanSeqScan(SeqScanState *node); #endif /* NODESEQSCAN_H */ diff --git a/src/include/executor/nodeTidscan.h b/src/include/executor/nodeTidscan.h index 813aab5d9f..d8679a00f5 100644 --- a/src/include/executor/nodeTidscan.h +++ b/src/include/executor/nodeTidscan.h @@ -19,8 +19,6 @@ extern TidScanState *ExecInitTidScan(TidScan *node, EState *estate, int eflags); extern TupleTableSlot *ExecTidScan(TidScanState *node); extern void ExecEndTidScan(TidScanState *node); -extern void ExecTidMarkPos(TidScanState *node); -extern void ExecTidRestrPos(TidScanState *node); extern void ExecReScanTidScan(TidScanState *node); #endif /* NODETIDSCAN_H */ diff --git a/src/include/executor/nodeValuesscan.h b/src/include/executor/nodeValuesscan.h index 4864d1a9ac..17bf399352 100644 --- a/src/include/executor/nodeValuesscan.h +++ b/src/include/executor/nodeValuesscan.h @@ -19,8 +19,6 @@ extern ValuesScanState *ExecInitValuesScan(ValuesScan *node, EState *estate, int eflags); extern TupleTableSlot *ExecValuesScan(ValuesScanState *node); extern void ExecEndValuesScan(ValuesScanState *node); -extern void ExecValuesMarkPos(ValuesScanState *node); -extern void ExecValuesRestrPos(ValuesScanState *node); extern void ExecReScanValuesScan(ValuesScanState *node); #endif /* NODEVALUESSCAN_H */ diff --git a/src/include/nodes/execnodes.h b/src/include/nodes/execnodes.h index 40fb8243ab..2bffa4be6c 100644 --- a/src/include/nodes/execnodes.h +++ b/src/include/nodes/execnodes.h @@ -1377,7 +1377,6 @@ typedef struct TidScanState bool tss_isCurrentOf; int tss_NumTids; int tss_TidPtr; - int tss_MarkTidPtr; ItemPointerData *tss_TidList; HeapTupleData tss_htup; } TidScanState; @@ -1435,7 +1434,6 @@ typedef struct FunctionScanState * exprlists array of expression lists being evaluated * array_len size of array * curr_idx current array index (0-based) - * marked_idx marked position (for mark/restore) * * Note: ss.ps.ps_ExprContext is used to evaluate any qual or projection * expressions attached to the node. We create a second ExprContext, @@ -1451,7 +1449,6 @@ typedef struct ValuesScanState List **exprlists; int array_len; int curr_idx; - int marked_idx; } ValuesScanState; /* ----------------