diff --git a/src/backend/access/heap/heapam.c b/src/backend/access/heap/heapam.c index e6024a980bb..0a8bac25f59 100644 --- a/src/backend/access/heap/heapam.c +++ b/src/backend/access/heap/heapam.c @@ -490,9 +490,6 @@ heapgetpage(TableScanDesc sscan, BlockNumber block) * tuple as indicated by "dir"; return the next tuple in scan->rs_ctup, * or set scan->rs_ctup.t_data = NULL if no more tuples. * - * dir == NoMovementScanDirection means "re-fetch the tuple indicated - * by scan->rs_ctup". - * * Note: the reason nkeys/key are passed separately, even though they are * kept in the scan descriptor, is that the caller may not want us to check * the scankeys. @@ -583,7 +580,7 @@ heapgettup(HeapScanDesc scan, linesleft = lines - lineoff + 1; } - else if (backward) + else { /* backward parallel scan not supported */ Assert(scan->rs_base.rs_parallel == NULL); @@ -653,34 +650,6 @@ heapgettup(HeapScanDesc scan, linesleft = lineoff; } - else - { - /* - * ``no movement'' scan direction: refetch prior tuple - */ - if (!scan->rs_inited) - { - Assert(!BufferIsValid(scan->rs_cbuf)); - tuple->t_data = NULL; - return; - } - - block = ItemPointerGetBlockNumber(&(tuple->t_self)); - if (block != scan->rs_cblock) - heapgetpage((TableScanDesc) scan, block); - - /* Since the tuple was previously fetched, needn't lock page here */ - page = BufferGetPage(scan->rs_cbuf); - TestForOldSnapshot(snapshot, scan->rs_base.rs_rd, page); - lineoff = ItemPointerGetOffsetNumber(&(tuple->t_self)); - lpp = PageGetItemId(page, lineoff); - Assert(ItemIdIsNormal(lpp)); - - tuple->t_data = (HeapTupleHeader) PageGetItem(page, lpp); - tuple->t_len = ItemIdGetLength(lpp); - - return; - } /* * advance the scan until we find a qualifying tuple or run out of stuff @@ -918,7 +887,7 @@ heapgettup_pagemode(HeapScanDesc scan, linesleft = lines - lineindex; } - else if (backward) + else { /* backward parallel scan not supported */ Assert(scan->rs_base.rs_parallel == NULL); @@ -978,38 +947,6 @@ heapgettup_pagemode(HeapScanDesc scan, linesleft = lineindex + 1; } - else - { - /* - * ``no movement'' scan direction: refetch prior tuple - */ - if (!scan->rs_inited) - { - Assert(!BufferIsValid(scan->rs_cbuf)); - tuple->t_data = NULL; - return; - } - - block = ItemPointerGetBlockNumber(&(tuple->t_self)); - if (block != scan->rs_cblock) - heapgetpage((TableScanDesc) scan, block); - - /* Since the tuple was previously fetched, needn't lock page here */ - page = BufferGetPage(scan->rs_cbuf); - TestForOldSnapshot(scan->rs_base.rs_snapshot, scan->rs_base.rs_rd, page); - lineoff = ItemPointerGetOffsetNumber(&(tuple->t_self)); - lpp = PageGetItemId(page, lineoff); - Assert(ItemIdIsNormal(lpp)); - - tuple->t_data = (HeapTupleHeader) PageGetItem(page, lpp); - tuple->t_len = ItemIdGetLength(lpp); - - /* check that rs_cindex is in sync */ - Assert(scan->rs_cindex < scan->rs_ntuples); - Assert(lineoff == scan->rs_vistuples[scan->rs_cindex]); - - return; - } /* * advance the scan until we find a qualifying tuple or run out of stuff diff --git a/src/backend/commands/explain.c b/src/backend/commands/explain.c index 35c23bd27df..fbbf28cf063 100644 --- a/src/backend/commands/explain.c +++ b/src/backend/commands/explain.c @@ -3746,9 +3746,6 @@ ExplainIndexScanDetails(Oid indexid, ScanDirection indexorderdir, case BackwardScanDirection: scandir = "Backward"; break; - case NoMovementScanDirection: - scandir = "NoMovement"; - break; case ForwardScanDirection: scandir = "Forward"; break; diff --git a/src/backend/executor/nodeIndexonlyscan.c b/src/backend/executor/nodeIndexonlyscan.c index 8c7da9ee60a..0b43a9b9699 100644 --- a/src/backend/executor/nodeIndexonlyscan.c +++ b/src/backend/executor/nodeIndexonlyscan.c @@ -70,15 +70,13 @@ IndexOnlyNext(IndexOnlyScanState *node) * extract necessary information from index scan node */ estate = node->ss.ps.state; - direction = estate->es_direction; - /* flip direction if this is an overall backward scan */ - if (ScanDirectionIsBackward(((IndexOnlyScan *) node->ss.ps.plan)->indexorderdir)) - { - if (ScanDirectionIsForward(direction)) - direction = BackwardScanDirection; - else if (ScanDirectionIsBackward(direction)) - direction = ForwardScanDirection; - } + + /* + * Determine which direction to scan the index in based on the plan's scan + * direction and the current direction of execution. + */ + direction = ScanDirectionCombine(estate->es_direction, + ((IndexOnlyScan *) node->ss.ps.plan)->indexorderdir); scandesc = node->ioss_ScanDesc; econtext = node->ss.ps.ps_ExprContext; slot = node->ss.ss_ScanTupleSlot; diff --git a/src/backend/executor/nodeIndexscan.c b/src/backend/executor/nodeIndexscan.c index f1ced9ff0fa..4540c7781d2 100644 --- a/src/backend/executor/nodeIndexscan.c +++ b/src/backend/executor/nodeIndexscan.c @@ -90,15 +90,13 @@ IndexNext(IndexScanState *node) * extract necessary information from index scan node */ estate = node->ss.ps.state; - direction = estate->es_direction; - /* flip direction if this is an overall backward scan */ - if (ScanDirectionIsBackward(((IndexScan *) node->ss.ps.plan)->indexorderdir)) - { - if (ScanDirectionIsForward(direction)) - direction = BackwardScanDirection; - else if (ScanDirectionIsBackward(direction)) - direction = ForwardScanDirection; - } + + /* + * Determine which direction to scan the index in based on the plan's scan + * direction and the current direction of execution. + */ + direction = ScanDirectionCombine(estate->es_direction, + ((IndexScan *) node->ss.ps.plan)->indexorderdir); scandesc = node->iss_ScanDesc; econtext = node->ss.ps.ps_ExprContext; slot = node->ss.ss_ScanTupleSlot; diff --git a/src/backend/optimizer/path/indxpath.c b/src/backend/optimizer/path/indxpath.c index e9b784bcab9..721a0752018 100644 --- a/src/backend/optimizer/path/indxpath.c +++ b/src/backend/optimizer/path/indxpath.c @@ -1015,9 +1015,7 @@ build_index_paths(PlannerInfo *root, RelOptInfo *rel, orderbyclauses, orderbyclausecols, useful_pathkeys, - index_is_ordered ? - ForwardScanDirection : - NoMovementScanDirection, + ForwardScanDirection, index_only_scan, outer_relids, loop_count, @@ -1037,9 +1035,7 @@ build_index_paths(PlannerInfo *root, RelOptInfo *rel, orderbyclauses, orderbyclausecols, useful_pathkeys, - index_is_ordered ? - ForwardScanDirection : - NoMovementScanDirection, + ForwardScanDirection, index_only_scan, outer_relids, loop_count, diff --git a/src/backend/optimizer/plan/createplan.c b/src/backend/optimizer/plan/createplan.c index 1b118528141..134130476e4 100644 --- a/src/backend/optimizer/plan/createplan.c +++ b/src/backend/optimizer/plan/createplan.c @@ -3017,6 +3017,9 @@ create_indexscan_plan(PlannerInfo *root, /* it should be a base rel... */ Assert(baserelid > 0); Assert(best_path->path.parent->rtekind == RTE_RELATION); + /* check the scan direction is valid */ + Assert(best_path->indexscandir == ForwardScanDirection || + best_path->indexscandir == BackwardScanDirection); /* * Extract the index qual expressions (stripped of RestrictInfos) from the diff --git a/src/backend/optimizer/util/pathnode.c b/src/backend/optimizer/util/pathnode.c index f2bf68d33be..d749b505785 100644 --- a/src/backend/optimizer/util/pathnode.c +++ b/src/backend/optimizer/util/pathnode.c @@ -982,9 +982,7 @@ create_samplescan_path(PlannerInfo *root, RelOptInfo *rel, Relids required_outer * 'indexorderbycols' is an integer list of index column numbers (zero based) * the ordering operators can be used with. * 'pathkeys' describes the ordering of the path. - * 'indexscandir' is ForwardScanDirection or BackwardScanDirection - * for an ordered index, or NoMovementScanDirection for - * an unordered index. + * 'indexscandir' is either ForwardScanDirection or BackwardScanDirection. * 'indexonly' is true if an index-only scan is wanted. * 'required_outer' is the set of outer relids for a parameterized path. * 'loop_count' is the number of repetitions of the indexscan to factor into diff --git a/src/include/access/sdir.h b/src/include/access/sdir.h index 16cb06c709b..322aeb3ff9d 100644 --- a/src/include/access/sdir.h +++ b/src/include/access/sdir.h @@ -16,8 +16,10 @@ /* - * ScanDirection was an int8 for no apparent reason. I kept the original - * values because I'm not sure if I'll break anything otherwise. -ay 2/95 + * Defines the direction for scanning a table or an index. Scans are never + * invoked using NoMovementScanDirectionScans. For convenience, we use the + * values -1 and 1 for backward and forward scans. This allows us to perform + * a few mathematical tricks such as what is done in ScanDirectionCombine. */ typedef enum ScanDirection { @@ -26,6 +28,13 @@ typedef enum ScanDirection ForwardScanDirection = 1 } ScanDirection; +/* + * Determine the net effect of two direction specifications. + * This relies on having ForwardScanDirection = +1, BackwardScanDirection = -1, + * and will probably not do what you want if applied to any other values. + */ +#define ScanDirectionCombine(a, b) ((a) * (b)) + /* * ScanDirectionIsValid * True iff scan direction is valid. diff --git a/src/include/access/tableam.h b/src/include/access/tableam.h index 3fb184717f6..652e96f1b0b 100644 --- a/src/include/access/tableam.h +++ b/src/include/access/tableam.h @@ -1035,6 +1035,10 @@ table_scan_getnextslot(TableScanDesc sscan, ScanDirection direction, TupleTableS { slot->tts_tableOid = RelationGetRelid(sscan->rs_rd); + /* We don't expect actual scans using NoMovementScanDirection */ + Assert(direction == ForwardScanDirection || + direction == BackwardScanDirection); + /* * We don't expect direct calls to table_scan_getnextslot with valid * CheckXidAlive for catalog or regular tables. See detailed comments in @@ -1099,6 +1103,10 @@ table_scan_getnextslot_tidrange(TableScanDesc sscan, ScanDirection direction, /* Ensure table_beginscan_tidrange() was used. */ Assert((sscan->rs_flags & SO_TYPE_TIDRANGESCAN) != 0); + /* We don't expect actual scans using NoMovementScanDirection */ + Assert(direction == ForwardScanDirection || + direction == BackwardScanDirection); + return sscan->rs_rd->rd_tableam->scan_getnextslot_tidrange(sscan, direction, slot); diff --git a/src/include/nodes/pathnodes.h b/src/include/nodes/pathnodes.h index 62d9460258c..0d4b1ec4e42 100644 --- a/src/include/nodes/pathnodes.h +++ b/src/include/nodes/pathnodes.h @@ -1672,12 +1672,9 @@ typedef struct Path * on which index column each ORDER BY can be used with.) * * 'indexscandir' is one of: - * ForwardScanDirection: forward scan of an ordered index + * ForwardScanDirection: forward scan of an index * BackwardScanDirection: backward scan of an ordered index - * NoMovementScanDirection: scan of an unordered index, or don't care - * (The executor doesn't care whether it gets ForwardScanDirection or - * NoMovementScanDirection for an indexscan, but the planner wants to - * distinguish ordered from unordered indexes for building pathkeys.) + * Unordered indexes will always have an indexscandir of ForwardScanDirection. * * 'indextotalcost' and 'indexselectivity' are saved in the IndexPath so that * we need not recompute them when considering using the same index in a