Skip to content

Commit e9aaf06

Browse files
david-rowleydavid-rowley
committed
Remove dead NoMovementScanDirection code
Here remove some dead code from heapgettup() and heapgettup_pagemode() which was trying to support NoMovementScanDirection scans. This code can never be reached as standard_ExecutorRun() never calls ExecutePlan with NoMovementScanDirection. Additionally, plans which were scanning an unordered index would use NoMovementScanDirection rather than ForwardScanDirection. There was no real need for this, so here we adjust this so we use ForwardScanDirection for unordered index scans. A comment in pathnodes.h claimed that NoMovementScanDirection was used for PathKey reasons, but if that was true, it no longer is, per code in build_index_paths(). This does change the non-text format of the EXPLAIN output so that unordered index scans now have a "Forward" scan direction rather than "NoMovement". The text format of EXPLAIN has not changed. Author: Melanie Plageman Reviewed-by: Tom Lane, David Rowley Discussion: https://postgr.es/m/CAAKRu_bvkhka0CZQun28KTqhuUh5ZqY=_T8QEqZqOL02rpi2bw@mail.gmail.com
1 parent 856ca51 commit e9aaf06

File tree

10 files changed

+43
-102
lines changed

10 files changed

+43
-102
lines changed

‎src/backend/access/heap/heapam.c

Lines changed: 2 additions & 65 deletions
Original file line numberDiff line numberDiff line change
@@ -490,9 +490,6 @@ heapgetpage(TableScanDesc sscan, BlockNumber block)
490490
* tuple as indicated by "dir"; return the next tuple in scan->rs_ctup,
491491
* or set scan->rs_ctup.t_data = NULL if no more tuples.
492492
*
493-
* dir == NoMovementScanDirection means "re-fetch the tuple indicated
494-
* by scan->rs_ctup".
495-
*
496493
* Note: the reason nkeys/key are passed separately, even though they are
497494
* kept in the scan descriptor, is that the caller may not want us to check
498495
* the scankeys.
@@ -583,7 +580,7 @@ heapgettup(HeapScanDesc scan,
583580

584581
linesleft = lines - lineoff + 1;
585582
}
586-
else if (backward)
583+
else
587584
{
588585
/* backward parallel scan not supported */
589586
Assert(scan->rs_base.rs_parallel == NULL);
@@ -653,34 +650,6 @@ heapgettup(HeapScanDesc scan,
653650

654651
linesleft = lineoff;
655652
}
656-
else
657-
{
658-
/*
659-
* ``no movement'' scan direction: refetch prior tuple
660-
*/
661-
if (!scan->rs_inited)
662-
{
663-
Assert(!BufferIsValid(scan->rs_cbuf));
664-
tuple->t_data = NULL;
665-
return;
666-
}
667-
668-
block = ItemPointerGetBlockNumber(&(tuple->t_self));
669-
if (block != scan->rs_cblock)
670-
heapgetpage((TableScanDesc) scan, block);
671-
672-
/* Since the tuple was previously fetched, needn't lock page here */
673-
page = BufferGetPage(scan->rs_cbuf);
674-
TestForOldSnapshot(snapshot, scan->rs_base.rs_rd, page);
675-
lineoff = ItemPointerGetOffsetNumber(&(tuple->t_self));
676-
lpp = PageGetItemId(page, lineoff);
677-
Assert(ItemIdIsNormal(lpp));
678-
679-
tuple->t_data = (HeapTupleHeader) PageGetItem(page, lpp);
680-
tuple->t_len = ItemIdGetLength(lpp);
681-
682-
return;
683-
}
684653

685654
/*
686655
* advance the scan until we find a qualifying tuple or run out of stuff
@@ -918,7 +887,7 @@ heapgettup_pagemode(HeapScanDesc scan,
918887

919888
linesleft = lines - lineindex;
920889
}
921-
else if (backward)
890+
else
922891
{
923892
/* backward parallel scan not supported */
924893
Assert(scan->rs_base.rs_parallel == NULL);
@@ -978,38 +947,6 @@ heapgettup_pagemode(HeapScanDesc scan,
978947

979948
linesleft = lineindex + 1;
980949
}
981-
else
982-
{
983-
/*
984-
* ``no movement'' scan direction: refetch prior tuple
985-
*/
986-
if (!scan->rs_inited)
987-
{
988-
Assert(!BufferIsValid(scan->rs_cbuf));
989-
tuple->t_data = NULL;
990-
return;
991-
}
992-
993-
block = ItemPointerGetBlockNumber(&(tuple->t_self));
994-
if (block != scan->rs_cblock)
995-
heapgetpage((TableScanDesc) scan, block);
996-
997-
/* Since the tuple was previously fetched, needn't lock page here */
998-
page = BufferGetPage(scan->rs_cbuf);
999-
TestForOldSnapshot(scan->rs_base.rs_snapshot, scan->rs_base.rs_rd, page);
1000-
lineoff = ItemPointerGetOffsetNumber(&(tuple->t_self));
1001-
lpp = PageGetItemId(page, lineoff);
1002-
Assert(ItemIdIsNormal(lpp));
1003-
1004-
tuple->t_data = (HeapTupleHeader) PageGetItem(page, lpp);
1005-
tuple->t_len = ItemIdGetLength(lpp);
1006-
1007-
/* check that rs_cindex is in sync */
1008-
Assert(scan->rs_cindex < scan->rs_ntuples);
1009-
Assert(lineoff == scan->rs_vistuples[scan->rs_cindex]);
1010-
1011-
return;
1012-
}
1013950

1014951
/*
1015952
* advance the scan until we find a qualifying tuple or run out of stuff

‎src/backend/commands/explain.c

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3746,9 +3746,6 @@ ExplainIndexScanDetails(Oid indexid, ScanDirection indexorderdir,
37463746
case BackwardScanDirection:
37473747
scandir = "Backward";
37483748
break;
3749-
case NoMovementScanDirection:
3750-
scandir = "NoMovement";
3751-
break;
37523749
case ForwardScanDirection:
37533750
scandir = "Forward";
37543751
break;

‎src/backend/executor/nodeIndexonlyscan.c

Lines changed: 7 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -70,15 +70,13 @@ IndexOnlyNext(IndexOnlyScanState *node)
7070
* extract necessary information from index scan node
7171
*/
7272
estate = node->ss.ps.state;
73-
direction = estate->es_direction;
74-
/* flip direction if this is an overall backward scan */
75-
if (ScanDirectionIsBackward(((IndexOnlyScan *) node->ss.ps.plan)->indexorderdir))
76-
{
77-
if (ScanDirectionIsForward(direction))
78-
direction = BackwardScanDirection;
79-
else if (ScanDirectionIsBackward(direction))
80-
direction = ForwardScanDirection;
81-
}
73+
74+
/*
75+
* Determine which direction to scan the index in based on the plan's scan
76+
* direction and the current direction of execution.
77+
*/
78+
direction = ScanDirectionCombine(estate->es_direction,
79+
((IndexOnlyScan *) node->ss.ps.plan)->indexorderdir);
8280
scandesc = node->ioss_ScanDesc;
8381
econtext = node->ss.ps.ps_ExprContext;
8482
slot = node->ss.ss_ScanTupleSlot;

‎src/backend/executor/nodeIndexscan.c

Lines changed: 7 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -90,15 +90,13 @@ IndexNext(IndexScanState *node)
9090
* extract necessary information from index scan node
9191
*/
9292
estate = node->ss.ps.state;
93-
direction = estate->es_direction;
94-
/* flip direction if this is an overall backward scan */
95-
if (ScanDirectionIsBackward(((IndexScan *) node->ss.ps.plan)->indexorderdir))
96-
{
97-
if (ScanDirectionIsForward(direction))
98-
direction = BackwardScanDirection;
99-
else if (ScanDirectionIsBackward(direction))
100-
direction = ForwardScanDirection;
101-
}
93+
94+
/*
95+
* Determine which direction to scan the index in based on the plan's scan
96+
* direction and the current direction of execution.
97+
*/
98+
direction = ScanDirectionCombine(estate->es_direction,
99+
((IndexScan *) node->ss.ps.plan)->indexorderdir);
102100
scandesc = node->iss_ScanDesc;
103101
econtext = node->ss.ps.ps_ExprContext;
104102
slot = node->ss.ss_ScanTupleSlot;

‎src/backend/optimizer/path/indxpath.c

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1015,9 +1015,7 @@ build_index_paths(PlannerInfo *root, RelOptInfo *rel,
10151015
orderbyclauses,
10161016
orderbyclausecols,
10171017
useful_pathkeys,
1018-
index_is_ordered ?
1019-
ForwardScanDirection :
1020-
NoMovementScanDirection,
1018+
ForwardScanDirection,
10211019
index_only_scan,
10221020
outer_relids,
10231021
loop_count,
@@ -1037,9 +1035,7 @@ build_index_paths(PlannerInfo *root, RelOptInfo *rel,
10371035
orderbyclauses,
10381036
orderbyclausecols,
10391037
useful_pathkeys,
1040-
index_is_ordered ?
1041-
ForwardScanDirection :
1042-
NoMovementScanDirection,
1038+
ForwardScanDirection,
10431039
index_only_scan,
10441040
outer_relids,
10451041
loop_count,

‎src/backend/optimizer/plan/createplan.c

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3017,6 +3017,9 @@ create_indexscan_plan(PlannerInfo *root,
30173017
/* it should be a base rel... */
30183018
Assert(baserelid > 0);
30193019
Assert(best_path->path.parent->rtekind == RTE_RELATION);
3020+
/* check the scan direction is valid */
3021+
Assert(best_path->indexscandir == ForwardScanDirection ||
3022+
best_path->indexscandir == BackwardScanDirection);
30203023

30213024
/*
30223025
* Extract the index qual expressions (stripped of RestrictInfos) from the

‎src/backend/optimizer/util/pathnode.c

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -982,9 +982,7 @@ create_samplescan_path(PlannerInfo *root, RelOptInfo *rel, Relids required_outer
982982
* 'indexorderbycols' is an integer list of index column numbers (zero based)
983983
* the ordering operators can be used with.
984984
* 'pathkeys' describes the ordering of the path.
985-
* 'indexscandir' is ForwardScanDirection or BackwardScanDirection
986-
* for an ordered index, or NoMovementScanDirection for
987-
* an unordered index.
985+
* 'indexscandir' is either ForwardScanDirection or BackwardScanDirection.
988986
* 'indexonly' is true if an index-only scan is wanted.
989987
* 'required_outer' is the set of outer relids for a parameterized path.
990988
* 'loop_count' is the number of repetitions of the indexscan to factor into

‎src/include/access/sdir.h

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -16,8 +16,10 @@
1616

1717

1818
/*
19-
* ScanDirection was an int8 for no apparent reason. I kept the original
20-
* values because I'm not sure if I'll break anything otherwise. -ay 2/95
19+
* Defines the direction for scanning a table or an index. Scans are never
20+
* invoked using NoMovementScanDirectionScans. For convenience, we use the
21+
* values -1 and 1 for backward and forward scans. This allows us to perform
22+
* a few mathematical tricks such as what is done in ScanDirectionCombine.
2123
*/
2224
typedef enum ScanDirection
2325
{
@@ -26,6 +28,13 @@ typedef enum ScanDirection
2628
ForwardScanDirection = 1
2729
} ScanDirection;
2830

31+
/*
32+
* Determine the net effect of two direction specifications.
33+
* This relies on having ForwardScanDirection = +1, BackwardScanDirection = -1,
34+
* and will probably not do what you want if applied to any other values.
35+
*/
36+
#define ScanDirectionCombine(a, b) ((a) * (b))
37+
2938
/*
3039
* ScanDirectionIsValid
3140
* True iff scan direction is valid.

‎src/include/access/tableam.h

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1035,6 +1035,10 @@ table_scan_getnextslot(TableScanDesc sscan, ScanDirection direction, TupleTableS
10351035
{
10361036
slot->tts_tableOid = RelationGetRelid(sscan->rs_rd);
10371037

1038+
/* We don't expect actual scans using NoMovementScanDirection */
1039+
Assert(direction == ForwardScanDirection ||
1040+
direction == BackwardScanDirection);
1041+
10381042
/*
10391043
* We don't expect direct calls to table_scan_getnextslot with valid
10401044
* CheckXidAlive for catalog or regular tables. See detailed comments in
@@ -1099,6 +1103,10 @@ table_scan_getnextslot_tidrange(TableScanDesc sscan, ScanDirection direction,
10991103
/* Ensure table_beginscan_tidrange() was used. */
11001104
Assert((sscan->rs_flags & SO_TYPE_TIDRANGESCAN) != 0);
11011105

1106+
/* We don't expect actual scans using NoMovementScanDirection */
1107+
Assert(direction == ForwardScanDirection ||
1108+
direction == BackwardScanDirection);
1109+
11021110
return sscan->rs_rd->rd_tableam->scan_getnextslot_tidrange(sscan,
11031111
direction,
11041112
slot);

‎src/include/nodes/pathnodes.h

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1672,12 +1672,9 @@ typedef struct Path
16721672
* on which index column each ORDER BY can be used with.)
16731673
*
16741674
* 'indexscandir' is one of:
1675-
* ForwardScanDirection: forward scan of an ordered index
1675+
* ForwardScanDirection: forward scan of an index
16761676
* BackwardScanDirection: backward scan of an ordered index
1677-
* NoMovementScanDirection: scan of an unordered index, or don't care
1678-
* (The executor doesn't care whether it gets ForwardScanDirection or
1679-
* NoMovementScanDirection for an indexscan, but the planner wants to
1680-
* distinguish ordered from unordered indexes for building pathkeys.)
1677+
* Unordered indexes will always have an indexscandir of ForwardScanDirection.
16811678
*
16821679
* 'indextotalcost' and 'indexselectivity' are saved in the IndexPath so that
16831680
* we need not recompute them when considering using the same index in a

0 commit comments

Comments
 (0)