From 840e5fa3e9ba505a772296bb42feda5429c88690 Mon Sep 17 00:00:00 2001 From: Robert Haas Date: Thu, 2 May 2024 11:18:44 -0400 Subject: [PATCH 2/4] Don't generate index-scan paths when enable_indexscan=false. Previously, index-scan paths were still generated even when enable_indexscan=false, but we added disable-cost to the cost of both index scan plans and index-only scan plans. It doesn't make sense for enable_indexscan to affect the whether index-only scans are chosen given that we also have a GUC called enable_indexonlyscan. With this commit, enable_indexscan and enable_indexonlyscan work the same way: each one prevents consideration of paths of the appropriate type, and neither has any affect on the cost of the generate paths. This requires some updates to the regression tests, which previously relied on enable_indexscan=false to also disable index-only scans. Note that when enable_indexonlyscan=false and enable_indexscan=true, we will generate index-scan paths that would have not have been generated if both had been set to true. That's because generating both an index-scan path and an index-only path would be a waste of cycles, since the index-only path should always win. In effect, the index-scan plan shape was still being considered; we just rejected it before actually constructing a path. --- src/backend/optimizer/path/costsize.c | 4 --- src/backend/optimizer/path/indxpath.c | 26 ++++++++++++++++--- src/test/regress/expected/btree_index.out | 3 +++ src/test/regress/expected/create_index.out | 2 ++ src/test/regress/expected/select.out | 1 + src/test/regress/expected/select_parallel.out | 2 ++ src/test/regress/expected/tuplesort.out | 2 ++ src/test/regress/sql/btree_index.sql | 5 ++++ src/test/regress/sql/create_index.sql | 2 ++ src/test/regress/sql/select.sql | 1 + src/test/regress/sql/select_parallel.sql | 2 ++ src/test/regress/sql/tuplesort.sql | 2 ++ 12 files changed, 44 insertions(+), 8 deletions(-) diff --git a/src/backend/optimizer/path/costsize.c b/src/backend/optimizer/path/costsize.c index 2021c481b4..74fc5aab56 100644 --- a/src/backend/optimizer/path/costsize.c +++ b/src/backend/optimizer/path/costsize.c @@ -603,10 +603,6 @@ cost_index(IndexPath *path, PlannerInfo *root, double loop_count, path->indexclauses); } - if (!enable_indexscan) - startup_cost += disable_cost; - /* we don't need to check enable_indexonlyscan; indxpath.c does that */ - /* * Call index-access-method-specific code to estimate the processing cost * for scanning the index, as well as the selectivity of the index (ie, diff --git a/src/backend/optimizer/path/indxpath.c b/src/backend/optimizer/path/indxpath.c index c0fcc7d78d..423099d725 100644 --- a/src/backend/optimizer/path/indxpath.c +++ b/src/backend/optimizer/path/indxpath.c @@ -742,7 +742,13 @@ get_index_paths(PlannerInfo *root, RelOptInfo *rel, IndexPath *ipath = (IndexPath *) lfirst(lc); if (index->amhasgettuple) - add_path(rel, (Path *) ipath); + { + if (ipath->path.pathtype == T_IndexScan && enable_indexscan) + add_path(rel, (Path *) ipath); + else if (ipath->path.pathtype == T_IndexOnlyScan && + enable_indexonlyscan) + add_path(rel, (Path *) ipath); + } if (index->amhasgetbitmap && (ipath->path.pathkeys == NIL || @@ -831,6 +837,8 @@ build_index_paths(PlannerInfo *root, RelOptInfo *rel, case ST_INDEXSCAN: if (!index->amhasgettuple) return NIL; + if (!enable_indexscan && !enable_indexonlyscan) + return NIL; break; case ST_BITMAPSCAN: if (!index->amhasgetbitmap) @@ -978,7 +986,8 @@ build_index_paths(PlannerInfo *root, RelOptInfo *rel, */ if (index->amcanparallel && rel->consider_parallel && outer_relids == NULL && - scantype != ST_BITMAPSCAN) + scantype != ST_BITMAPSCAN && + (index_only_scan ? enable_indexonlyscan : enable_indexscan)) { ipath = create_index_path(root, index, index_clauses, @@ -1028,7 +1037,8 @@ build_index_paths(PlannerInfo *root, RelOptInfo *rel, /* If appropriate, consider parallel index scan */ if (index->amcanparallel && rel->consider_parallel && outer_relids == NULL && - scantype != ST_BITMAPSCAN) + scantype != ST_BITMAPSCAN && + (index_only_scan ? enable_indexonlyscan : enable_indexscan)) { ipath = create_index_path(root, index, index_clauses, @@ -1735,7 +1745,15 @@ check_index_only(RelOptInfo *rel, IndexOptInfo *index) ListCell *lc; int i; - /* Index-only scans must be enabled */ + /* + * Index-only scans must be enabled. + * + * NB: Returning false here means that an index scan will be considered + * instead, so setting enable_indexscan=false causes to consider paths + * that we wouldn't have considered otherwise. That seems OK, because our + * only reason for not generating the index-scan paths is that we expect + * them to lose on cost. + */ if (!enable_indexonlyscan) return false; diff --git a/src/test/regress/expected/btree_index.out b/src/test/regress/expected/btree_index.out index 510646cbce..f15db99771 100644 --- a/src/test/regress/expected/btree_index.out +++ b/src/test/regress/expected/btree_index.out @@ -247,6 +247,7 @@ select thousand from tenk1 where thousand in (364, 366,380) and tenthous = 20000 -- set enable_seqscan to false; set enable_indexscan to true; +set enable_indexonlyscan to true; set enable_bitmapscan to false; explain (costs off) select proname from pg_proc where proname like E'RI\\_FKey%del' order by 1; @@ -290,6 +291,7 @@ select proname from pg_proc where proname ilike 'ri%foo' order by 1; (2 rows) set enable_indexscan to false; +set enable_indexonlyscan to false; set enable_bitmapscan to true; explain (costs off) select proname from pg_proc where proname like E'RI\\_FKey%del' order by 1; @@ -330,6 +332,7 @@ select proname from pg_proc where proname ilike '00%foo' order by 1; --------- (0 rows) +reset enable_indexonlyscan; explain (costs off) select proname from pg_proc where proname ilike 'ri%foo' order by 1; QUERY PLAN diff --git a/src/test/regress/expected/create_index.out b/src/test/regress/expected/create_index.out index cf6eac5734..ec69bafd40 100644 --- a/src/test/regress/expected/create_index.out +++ b/src/test/regress/expected/create_index.out @@ -618,6 +618,7 @@ SELECT point(x,x), (SELECT f1 FROM gpolygon_tbl ORDER BY f1 <-> point(x,x) LIMIT -- Now check the results from bitmap indexscan SET enable_seqscan = OFF; SET enable_indexscan = OFF; +SET enable_indexonlyscan = OFF; SET enable_bitmapscan = ON; EXPLAIN (COSTS OFF) SELECT * FROM point_tbl WHERE f1 <@ '(-10,-10),(10,10)':: box ORDER BY f1 <-> '0,1'; @@ -643,6 +644,7 @@ SELECT * FROM point_tbl WHERE f1 <@ '(-10,-10),(10,10)':: box ORDER BY f1 <-> '0 RESET enable_seqscan; RESET enable_indexscan; +RESET enable_indexonlyscan; RESET enable_bitmapscan; -- -- GIN over int[] and text[] diff --git a/src/test/regress/expected/select.out b/src/test/regress/expected/select.out index 33a6dceb0e..6445815741 100644 --- a/src/test/regress/expected/select.out +++ b/src/test/regress/expected/select.out @@ -844,6 +844,7 @@ select unique2 from onek2 where unique2 = 11 and stringu1 < 'C'; -- partial index implies clause, but bitmap scan must recheck predicate anyway SET enable_indexscan TO off; +SET enable_indexonlyscan TO off; explain (costs off) select unique2 from onek2 where unique2 = 11 and stringu1 < 'B'; QUERY PLAN diff --git a/src/test/regress/expected/select_parallel.out b/src/test/regress/expected/select_parallel.out index 87273fa635..f79eda79f6 100644 --- a/src/test/regress/expected/select_parallel.out +++ b/src/test/regress/expected/select_parallel.out @@ -522,6 +522,7 @@ reset enable_indexscan; -- test parallel bitmap heap scan. set enable_seqscan to off; set enable_indexscan to off; +set enable_indexonlyscan to off; set enable_hashjoin to off; set enable_mergejoin to off; set enable_material to off; @@ -622,6 +623,7 @@ select * from explain_parallel_sort_stats(); (14 rows) reset enable_indexscan; +reset enable_indexonlyscan; reset enable_hashjoin; reset enable_mergejoin; reset enable_material; diff --git a/src/test/regress/expected/tuplesort.out b/src/test/regress/expected/tuplesort.out index 6dd97e7427..87b05a22cb 100644 --- a/src/test/regress/expected/tuplesort.out +++ b/src/test/regress/expected/tuplesort.out @@ -362,6 +362,7 @@ ORDER BY v.a DESC; -- in-memory BEGIN; SET LOCAL enable_indexscan = false; +SET LOCAL enable_indexonlyscan = false; -- unfortunately can't show analyze output confirming sort method, -- the memory used output wouldn't be stable EXPLAIN (COSTS OFF) DECLARE c SCROLL CURSOR FOR SELECT noabort_decreasing FROM abbrev_abort_uuids ORDER BY noabort_decreasing; @@ -458,6 +459,7 @@ COMMIT; -- disk based BEGIN; SET LOCAL enable_indexscan = false; +SET LOCAL enable_indexonlyscan = false; SET LOCAL work_mem = '100kB'; -- unfortunately can't show analyze output confirming sort method, -- the memory used output wouldn't be stable diff --git a/src/test/regress/sql/btree_index.sql b/src/test/regress/sql/btree_index.sql index 0d2a33f370..bc99f44dda 100644 --- a/src/test/regress/sql/btree_index.sql +++ b/src/test/regress/sql/btree_index.sql @@ -157,6 +157,7 @@ select thousand from tenk1 where thousand in (364, 366,380) and tenthous = 20000 set enable_seqscan to false; set enable_indexscan to true; +set enable_indexonlyscan to true; set enable_bitmapscan to false; explain (costs off) select proname from pg_proc where proname like E'RI\\_FKey%del' order by 1; @@ -168,6 +169,7 @@ explain (costs off) select proname from pg_proc where proname ilike 'ri%foo' order by 1; set enable_indexscan to false; +set enable_indexonlyscan to false; set enable_bitmapscan to true; explain (costs off) select proname from pg_proc where proname like E'RI\\_FKey%del' order by 1; @@ -175,6 +177,9 @@ select proname from pg_proc where proname like E'RI\\_FKey%del' order by 1; explain (costs off) select proname from pg_proc where proname ilike '00%foo' order by 1; select proname from pg_proc where proname ilike '00%foo' order by 1; + +reset enable_indexonlyscan; + explain (costs off) select proname from pg_proc where proname ilike 'ri%foo' order by 1; diff --git a/src/test/regress/sql/create_index.sql b/src/test/regress/sql/create_index.sql index e296891cab..04dea5225e 100644 --- a/src/test/regress/sql/create_index.sql +++ b/src/test/regress/sql/create_index.sql @@ -246,6 +246,7 @@ SELECT point(x,x), (SELECT f1 FROM gpolygon_tbl ORDER BY f1 <-> point(x,x) LIMIT -- Now check the results from bitmap indexscan SET enable_seqscan = OFF; SET enable_indexscan = OFF; +SET enable_indexonlyscan = OFF; SET enable_bitmapscan = ON; EXPLAIN (COSTS OFF) @@ -254,6 +255,7 @@ SELECT * FROM point_tbl WHERE f1 <@ '(-10,-10),(10,10)':: box ORDER BY f1 <-> '0 RESET enable_seqscan; RESET enable_indexscan; +RESET enable_indexonlyscan; RESET enable_bitmapscan; -- diff --git a/src/test/regress/sql/select.sql b/src/test/regress/sql/select.sql index 019f1e7673..a0c7417dec 100644 --- a/src/test/regress/sql/select.sql +++ b/src/test/regress/sql/select.sql @@ -218,6 +218,7 @@ select unique2 from onek2 where unique2 = 11 and stringu1 < 'C'; select unique2 from onek2 where unique2 = 11 and stringu1 < 'C'; -- partial index implies clause, but bitmap scan must recheck predicate anyway SET enable_indexscan TO off; +SET enable_indexonlyscan TO off; explain (costs off) select unique2 from onek2 where unique2 = 11 and stringu1 < 'B'; select unique2 from onek2 where unique2 = 11 and stringu1 < 'B'; diff --git a/src/test/regress/sql/select_parallel.sql b/src/test/regress/sql/select_parallel.sql index 20376c03fa..3f003e2e71 100644 --- a/src/test/regress/sql/select_parallel.sql +++ b/src/test/regress/sql/select_parallel.sql @@ -201,6 +201,7 @@ reset enable_indexscan; -- test parallel bitmap heap scan. set enable_seqscan to off; set enable_indexscan to off; +set enable_indexonlyscan to off; set enable_hashjoin to off; set enable_mergejoin to off; set enable_material to off; @@ -248,6 +249,7 @@ $$; select * from explain_parallel_sort_stats(); reset enable_indexscan; +reset enable_indexonlyscan; reset enable_hashjoin; reset enable_mergejoin; reset enable_material; diff --git a/src/test/regress/sql/tuplesort.sql b/src/test/regress/sql/tuplesort.sql index 8476e594e6..95ac8ec04c 100644 --- a/src/test/regress/sql/tuplesort.sql +++ b/src/test/regress/sql/tuplesort.sql @@ -162,6 +162,7 @@ ORDER BY v.a DESC; -- in-memory BEGIN; SET LOCAL enable_indexscan = false; +SET LOCAL enable_indexonlyscan = false; -- unfortunately can't show analyze output confirming sort method, -- the memory used output wouldn't be stable EXPLAIN (COSTS OFF) DECLARE c SCROLL CURSOR FOR SELECT noabort_decreasing FROM abbrev_abort_uuids ORDER BY noabort_decreasing; @@ -192,6 +193,7 @@ COMMIT; -- disk based BEGIN; SET LOCAL enable_indexscan = false; +SET LOCAL enable_indexonlyscan = false; SET LOCAL work_mem = '100kB'; -- unfortunately can't show analyze output confirming sort method, -- the memory used output wouldn't be stable -- 2.39.3 (Apple Git-145)