Skip to content

Commit

Permalink
Revert pre 5bf748b SAOP behavior in PG17
Browse files Browse the repository at this point in the history
Commit 5bf748b allows generation of unsafe SAOP path keys
on a multicolumn index that were disabled previously by 807a40c.
  • Loading branch information
pashkinelfe committed Oct 21, 2024
1 parent 20cea79 commit 83d892d
Show file tree
Hide file tree
Showing 2 changed files with 52 additions and 14 deletions.
42 changes: 38 additions & 4 deletions src/backend/optimizer/path/indxpath.c
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,8 @@ static List *build_index_paths(PlannerInfo *root, RelOptInfo *rel,
IndexOptInfo *index, IndexClauseSet *clauses,
bool useful_predicate,
ScanTypeControl scantype,
bool *skip_nonnative_saop);
bool *skip_nonnative_saop,
bool *skip_lower_saop);
static List *build_paths_for_OR(PlannerInfo *root, RelOptInfo *rel,
List *clauses, List *other_clauses);
static List *generate_bitmap_or_paths(PlannerInfo *root, RelOptInfo *rel,
Expand Down Expand Up @@ -702,6 +703,7 @@ get_index_paths(PlannerInfo *root, RelOptInfo *rel,
{
List *indexpaths;
bool skip_nonnative_saop = false;
bool skip_lower_saop = false;
ListCell *lc;

/*
Expand All @@ -712,8 +714,24 @@ get_index_paths(PlannerInfo *root, RelOptInfo *rel,
index, clauses,
index->predOK,
ST_ANYSCAN,
&skip_nonnative_saop);

&skip_nonnative_saop,
&skip_lower_saop);

/*
* If we skipped any lower-order ScalarArrayOpExprs on an index with an AM
* that supports them, then try again including those clauses. This will
* produce paths with more selectivity but no ordering.
*/
if (skip_lower_saop)
{
indexpaths = list_concat(indexpaths,
build_index_paths(root, rel,
index, clauses,
index->predOK,
ST_ANYSCAN,
&skip_nonnative_saop,
NULL));
}
/*
* Submit all the ones that can form plain IndexScan plans to add_path. (A
* plain IndexPath can represent either a plain IndexScan or an
Expand Down Expand Up @@ -750,6 +768,7 @@ get_index_paths(PlannerInfo *root, RelOptInfo *rel,
index, clauses,
false,
ST_BITMAPSCAN,
NULL,
NULL);
*bitindexpaths = list_concat(*bitindexpaths, indexpaths);
}
Expand Down Expand Up @@ -794,7 +813,8 @@ build_index_paths(PlannerInfo *root, RelOptInfo *rel,
IndexOptInfo *index, IndexClauseSet *clauses,
bool useful_predicate,
ScanTypeControl scantype,
bool *skip_nonnative_saop)
bool *skip_nonnative_saop,
bool *skip_lower_saop)
{
List *result = NIL;
IndexPath *ipath;
Expand All @@ -805,6 +825,7 @@ build_index_paths(PlannerInfo *root, RelOptInfo *rel,
List *orderbyclausecols;
List *index_pathkeys;
List *useful_pathkeys;
bool found_lower_saop_clause;
bool pathkeys_possibly_useful;
bool index_is_ordered;
bool index_only_scan;
Expand Down Expand Up @@ -843,6 +864,7 @@ build_index_paths(PlannerInfo *root, RelOptInfo *rel,
* otherwise accounted for.
*/
index_clauses = NIL;
found_lower_saop_clause = false;
outer_relids = bms_copy(rel->lateral_relids);
for (indexcol = 0; indexcol < index->nkeycolumns; indexcol++)
{
Expand All @@ -866,6 +888,16 @@ build_index_paths(PlannerInfo *root, RelOptInfo *rel,
*skip_nonnative_saop = true;
continue;
}
if (skip_nonnative_saop && IsA(rinfo->clause, ScalarArrayOpExpr) && indexcol > 0)
{
if (skip_lower_saop)
{
/* Caller doesn't want to lose index ordering */
*skip_lower_saop = true;
continue;
}
found_lower_saop_clause = true;
}

/* OK to include this clause */
index_clauses = lappend(index_clauses, iclause);
Expand Down Expand Up @@ -897,6 +929,7 @@ build_index_paths(PlannerInfo *root, RelOptInfo *rel,
* if we are only trying to build bitmap indexscans.
*/
pathkeys_possibly_useful = (scantype != ST_BITMAPSCAN &&
!found_lower_saop_clause &&
has_useful_pathkeys(root, rel));
index_is_ordered = (index->sortopfamily != NULL);
if (index_is_ordered && pathkeys_possibly_useful)
Expand Down Expand Up @@ -1148,6 +1181,7 @@ build_paths_for_OR(PlannerInfo *root, RelOptInfo *rel,
index, &clauseset,
useful_predicate,
ST_BITMAPSCAN,
NULL,
NULL);
result = list_concat(result, indexpaths);
}
Expand Down
24 changes: 14 additions & 10 deletions src/test/regress/expected/create_index.out
Original file line number Diff line number Diff line change
Expand Up @@ -1943,11 +1943,13 @@ explain (costs off)
SELECT thousand, tenthous FROM tenk1
WHERE thousand < 2 AND tenthous IN (1001,3000)
ORDER BY thousand;
QUERY PLAN
--------------------------------------------------------------------------------
Index Only Scan using tenk1_thous_tenthous on tenk1
Index Cond: ((thousand < 2) AND (tenthous = ANY ('{1001,3000}'::integer[])))
(2 rows)
QUERY PLAN
--------------------------------------------------------------------------------------
Sort
Sort Key: thousand
-> Index Only Scan using tenk1_thous_tenthous on tenk1
Index Cond: ((thousand < 2) AND (tenthous = ANY ('{1001,3000}'::integer[])))
(4 rows)

SELECT thousand, tenthous FROM tenk1
WHERE thousand < 2 AND tenthous IN (1001,3000)
Expand All @@ -1963,11 +1965,13 @@ explain (costs off)
SELECT thousand, tenthous FROM tenk1
WHERE thousand < 2 AND tenthous IN (1001,3000)
ORDER BY thousand DESC, tenthous DESC;
QUERY PLAN
--------------------------------------------------------------------------------
Index Only Scan Backward using tenk1_thous_tenthous on tenk1
Index Cond: ((thousand < 2) AND (tenthous = ANY ('{1001,3000}'::integer[])))
(2 rows)
QUERY PLAN
--------------------------------------------------------------------------------------
Sort
Sort Key: thousand DESC, tenthous DESC
-> Index Only Scan using tenk1_thous_tenthous on tenk1
Index Cond: ((thousand < 2) AND (tenthous = ANY ('{1001,3000}'::integer[])))
(4 rows)

SELECT thousand, tenthous FROM tenk1
WHERE thousand < 2 AND tenthous IN (1001,3000)
Expand Down

0 comments on commit 83d892d

Please sign in to comment.