Thanks, for suggestions. On Sun 02. 07. 2023 at 10:18 Richard Guo <guofengli...@gmail.com> wrote: > 1. For comment "On success, the result list is ordered by pathkeys.", I > think it'd be more accurate if we say something like "On success, the > result list is ordered by pathkeys or a prefix list of pathkeys." > considering the possibility of incremental sort. > > 2. The comment below is not true anymore. > > /* > * Note: for any failure to match, we just return NIL immediately. > * There is no value in matching just some of the pathkeys. > */ > We should either remove it or change it to emphasize that we may return > a prefix of the pathkeys for incremental sort.
Comments are updated now. > BTW, would you please add the patch to the CF to not lose track of it? Submitted <https://commitfest.postgresql.org/43/4433/> -- Best regards Miroslav
diff --git a/src/backend/optimizer/path/indxpath.c b/src/backend/optimizer/path/indxpath.c index 0065c8992b..a1daa31d07 100644 --- a/src/backend/optimizer/path/indxpath.c +++ b/src/backend/optimizer/path/indxpath.c @@ -978,10 +978,8 @@ build_index_paths(PlannerInfo *root, RelOptInfo *rel, match_pathkeys_to_index(index, root->query_pathkeys, &orderbyclauses, &orderbyclausecols); - if (orderbyclauses) - useful_pathkeys = root->query_pathkeys; - else - useful_pathkeys = NIL; + useful_pathkeys = list_copy_head(root->query_pathkeys, + list_length(orderbyclauses)); } else { @@ -3059,16 +3057,14 @@ expand_indexqual_rowcompare(PlannerInfo *root, * index column numbers (zero based) that each clause would be used with. * NIL lists are returned if the ordering is not achievable this way. * - * On success, the result list is ordered by pathkeys, and in fact is - * one-to-one with the requested pathkeys. + * On success, the result list is ordered by pathkeys or a prefix list of + * pathkeys. Result can be shorter than pathkeys on partial prefix match. */ static void match_pathkeys_to_index(IndexOptInfo *index, List *pathkeys, List **orderby_clauses_p, List **clause_columns_p) { - List *orderby_clauses = NIL; - List *clause_columns = NIL; ListCell *lc1; *orderby_clauses_p = NIL; /* set default results */ @@ -3085,8 +3081,8 @@ match_pathkeys_to_index(IndexOptInfo *index, List *pathkeys, ListCell *lc2; /* - * Note: for any failure to match, we just return NIL immediately. - * There is no value in matching just some of the pathkeys. + * Note: for any failure to match, we just stop matching. Current + * longest match is already accumulated. */ /* Pathkey must request default sort order for the target opfamily */ @@ -3133,8 +3129,8 @@ match_pathkeys_to_index(IndexOptInfo *index, List *pathkeys, pathkey->pk_opfamily); if (expr) { - orderby_clauses = lappend(orderby_clauses, expr); - clause_columns = lappend_int(clause_columns, indexcol); + *orderby_clauses_p = lappend(*orderby_clauses_p, expr); + *clause_columns_p = lappend_int(*clause_columns_p, indexcol); found = true; break; } @@ -3144,12 +3140,9 @@ match_pathkeys_to_index(IndexOptInfo *index, List *pathkeys, break; } - if (!found) /* fail if no match for this pathkey */ + if (!found) /* end after first not matching expression */ return; } - - *orderby_clauses_p = orderby_clauses; /* success! */ - *clause_columns_p = clause_columns; } /* diff --git a/src/test/regress/expected/incremental_sort.out b/src/test/regress/expected/incremental_sort.out index 0c3433f8e5..41c62e0268 100644 --- a/src/test/regress/expected/incremental_sort.out +++ b/src/test/regress/expected/incremental_sort.out @@ -1660,3 +1660,33 @@ order by 1, 2; -> Function Scan on generate_series (7 rows) +-- Incremental sort for not ordered indexes, but with AM order operator +set enable_incremental_sort = on; +-- table with GiST index supports closest points retrieval, +-- but has not sorted index +create table t (a point, b int); +create index on t using gist(a); +insert into t select point(mod(i, 7), mod(i, 11)), i from generate_series(1, 1000) s(i); +analyze t; +explain (costs off) select a, b, a <-> point(5, 5) dist from t order by dist, b limit 1; + QUERY PLAN +------------------------------------------------- + Limit + -> Incremental Sort + Sort Key: ((a <-> '(5,5)'::point)), b + Presorted Key: ((a <-> '(5,5)'::point)) + -> Index Scan using t_a_idx on t + Order By: (a <-> '(5,5)'::point) +(6 rows) + +explain (costs off) select a, b, a <-> point(5, 5) dist from t order by dist, b desc limit 1; + QUERY PLAN +---------------------------------------------------- + Limit + -> Incremental Sort + Sort Key: ((a <-> '(5,5)'::point)), b DESC + Presorted Key: ((a <-> '(5,5)'::point)) + -> Index Scan using t_a_idx on t + Order By: (a <-> '(5,5)'::point) +(6 rows) + diff --git a/src/test/regress/sql/incremental_sort.sql b/src/test/regress/sql/incremental_sort.sql index 071f8a5268..c89e8e7409 100644 --- a/src/test/regress/sql/incremental_sort.sql +++ b/src/test/regress/sql/incremental_sort.sql @@ -276,3 +276,14 @@ from tenk1, lateral (select tenk1.unique1 from generate_series(1, 1000)) as sub; explain (costs off) select sub.unique1, stringu1 || random()::text from tenk1, lateral (select tenk1.unique1 from generate_series(1, 1000)) as sub order by 1, 2; + +-- Incremental sort for not ordered indexes, but with AM order operator +set enable_incremental_sort = on; +-- table with GiST index supports closest points retrieval, +-- but has not sorted index +create table t (a point, b int); +create index on t using gist(a); +insert into t select point(mod(i, 7), mod(i, 11)), i from generate_series(1, 1000) s(i); +analyze t; +explain (costs off) select a, b, a <-> point(5, 5) dist from t order by dist, b limit 1; +explain (costs off) select a, b, a <-> point(5, 5) dist from t order by dist, b desc limit 1;