From 0bd1749cffc389fb38111903f38f18e84218e5d1 Mon Sep 17 00:00:00 2001 From: Floris van Nee Date: Sun, 12 Jul 2020 13:36:25 +0200 Subject: [PATCH 7/7] planner fixes --- src/backend/optimizer/path/indxpath.c | 44 +++++++++++---- src/backend/optimizer/path/pathkeys.c | 72 +++++++++++++++++++++++++ src/backend/optimizer/path/uniquekeys.c | 17 +++--- src/backend/optimizer/plan/planner.c | 14 +++-- src/backend/optimizer/util/pathnode.c | 36 ++----------- src/include/optimizer/pathnode.h | 2 +- src/include/optimizer/paths.h | 4 ++ src/test/regress/expected/sysviews.out | 2 +- 8 files changed, 129 insertions(+), 62 deletions(-) diff --git a/src/backend/optimizer/path/indxpath.c b/src/backend/optimizer/path/indxpath.c index 340bbfcbb3..9eff71e5d5 100644 --- a/src/backend/optimizer/path/indxpath.c +++ b/src/backend/optimizer/path/indxpath.c @@ -1119,15 +1119,27 @@ build_index_paths(PlannerInfo *root, RelOptInfo *rel, /* Consider index skip scan as well */ if (root->query_uniquekeys != NULL && can_skip && !not_empty_qual) { - ListCell *lc; + int numusefulkeys = list_length(useful_pathkeys); + int numsortkeys = list_length(root->query_pathkeys); - foreach(lc, root->query_uniquekeys) + if (numusefulkeys == numsortkeys) { - UniqueKey *ukey = lfirst_node(UniqueKey, lc); + int prefix; + if (list_length(root->distinct_pathkeys) > 0) + prefix = find_index_prefix_for_pathkey(root, + index, + ForwardScanDirection, + llast_node(PathKey, + root->distinct_pathkeys)); + else + /* all are distinct keys are constant and optimized away. + * skipping with 1 is sufficient as all are constant anyway + */ + prefix = 1; + result = lappend(result, create_skipscan_unique_path(root, index, - (Path *) ipath, - ukey->exprs)); + (Path *) ipath, prefix)); } } @@ -1189,15 +1201,27 @@ build_index_paths(PlannerInfo *root, RelOptInfo *rel, /* Consider index skip scan as well */ if (root->query_uniquekeys != NULL && can_skip && !not_empty_qual) { - ListCell *lc; + int numusefulkeys = list_length(useful_pathkeys); + int numsortkeys = list_length(root->query_pathkeys); - foreach(lc, root->query_uniquekeys) + if (numusefulkeys == numsortkeys) { - UniqueKey *ukey = lfirst_node(UniqueKey, lc); + int prefix; + if (list_length(root->distinct_pathkeys) > 0) + prefix = find_index_prefix_for_pathkey(root, + index, + BackwardScanDirection, + llast_node(PathKey, + root->distinct_pathkeys)); + else + /* all are distinct keys are constant and optimized away. + * skipping with 1 is sufficient as all are constant anyway + */ + prefix = 1; + result = lappend(result, create_skipscan_unique_path(root, index, - (Path *) ipath, - ukey->exprs)); + (Path *) ipath, prefix)); } } diff --git a/src/backend/optimizer/path/pathkeys.c b/src/backend/optimizer/path/pathkeys.c index a4fc4f252d..3fa533be95 100644 --- a/src/backend/optimizer/path/pathkeys.c +++ b/src/backend/optimizer/path/pathkeys.c @@ -522,6 +522,78 @@ get_cheapest_parallel_safe_total_inner(List *paths) * NEW PATHKEY FORMATION ****************************************************************************/ +/* + * Find the prefix size for a specific path key in an index. + * For example, an index with (a,b,c) finding path key b will + * return prefix 2. + * Returns 0 when not found. + */ +int +find_index_prefix_for_pathkey(PlannerInfo *root, + IndexOptInfo *index, + ScanDirection scandir, + PathKey *pathkey) +{ + ListCell *lc; + int i; + + i = 0; + foreach(lc, index->indextlist) + { + TargetEntry *indextle = (TargetEntry *) lfirst(lc); + Expr *indexkey; + bool reverse_sort; + bool nulls_first; + PathKey *cpathkey; + + /* + * INCLUDE columns are stored in index unordered, so they don't + * support ordered index scan. + */ + if (i >= index->nkeycolumns) + break; + + /* We assume we don't need to make a copy of the tlist item */ + indexkey = indextle->expr; + + if (ScanDirectionIsBackward(scandir)) + { + reverse_sort = !index->reverse_sort[i]; + nulls_first = !index->nulls_first[i]; + } + else + { + reverse_sort = index->reverse_sort[i]; + nulls_first = index->nulls_first[i]; + } + + /* + * OK, try to make a canonical pathkey for this sort key. Note we're + * underneath any outer joins, so nullable_relids should be NULL. + */ + cpathkey = make_pathkey_from_sortinfo(root, + indexkey, + NULL, + index->sortopfamily[i], + index->opcintype[i], + index->indexcollations[i], + reverse_sort, + nulls_first, + 0, + index->rel->relids, + false); + + if (cpathkey == pathkey) + { + return i + 1; + } + + i++; + } + + return 0; +} + /* * build_index_pathkeys * Build a pathkeys list that describes the ordering induced by an index diff --git a/src/backend/optimizer/path/uniquekeys.c b/src/backend/optimizer/path/uniquekeys.c index 7d9c0aefbf..c02a3c0ed8 100644 --- a/src/backend/optimizer/path/uniquekeys.c +++ b/src/backend/optimizer/path/uniquekeys.c @@ -1136,6 +1136,7 @@ build_uniquekeys(PlannerInfo *root, List *sortclauses) List *result = NIL; List *sortkeys; ListCell *l; + List *exprs = NIL; sortkeys = make_pathkeys_for_uniquekeys(root, sortclauses, @@ -1146,18 +1147,14 @@ build_uniquekeys(PlannerInfo *root, List *sortclauses) { PathKey *pathkey = (PathKey *) lfirst(l); EquivalenceClass *ec = pathkey->pk_eclass; - ListCell *k; - List *exprs = NIL; - - foreach(k, ec->ec_members) - { - EquivalenceMember *mem = (EquivalenceMember *) lfirst(k); - exprs = lappend(exprs, mem->em_expr); - } - - result = lappend(result, makeUniqueKey(exprs, false, false)); + EquivalenceMember *mem = (EquivalenceMember*) lfirst(list_head(ec->ec_members)); + if (EC_MUST_BE_REDUNDANT(ec)) + continue; + exprs = lappend(exprs, mem->em_expr); } + result = lappend(result, makeUniqueKey(exprs, false)); + return result; } diff --git a/src/backend/optimizer/plan/planner.c b/src/backend/optimizer/plan/planner.c index a5f3c28fb5..80fbaadd21 100644 --- a/src/backend/optimizer/plan/planner.c +++ b/src/backend/optimizer/plan/planner.c @@ -3627,12 +3627,18 @@ standard_qp_callback(PlannerInfo *root, void *extra) if (parse->distinctClause && grouping_is_sortable(parse->distinctClause)) + { root->distinct_pathkeys = make_pathkeys_for_sortclauses(root, parse->distinctClause, tlist); + root->query_uniquekeys = build_uniquekeys(root, parse->distinctClause); + } else + { root->distinct_pathkeys = NIL; + root->query_uniquekeys = NIL; + } root->sort_pathkeys = make_pathkeys_for_sortclauses(root, @@ -3663,17 +3669,9 @@ standard_qp_callback(PlannerInfo *root, void *extra) root->query_pathkeys = root->window_pathkeys; else if (list_length(root->distinct_pathkeys) > list_length(root->sort_pathkeys)) - { root->query_pathkeys = root->distinct_pathkeys; - root->query_uniquekeys = build_uniquekeys(root, parse->distinctClause); - } else if (root->sort_pathkeys) - { root->query_pathkeys = root->sort_pathkeys; - - if (root->distinct_pathkeys) - root->query_uniquekeys = build_uniquekeys(root, parse->distinctClause); - } else root->query_pathkeys = NIL; } diff --git a/src/backend/optimizer/util/pathnode.c b/src/backend/optimizer/util/pathnode.c index cac2ca0b0f..340e8384e6 100644 --- a/src/backend/optimizer/util/pathnode.c +++ b/src/backend/optimizer/util/pathnode.c @@ -2981,49 +2981,21 @@ create_upper_unique_path(PlannerInfo *root, */ IndexPath * create_skipscan_unique_path(PlannerInfo *root, IndexOptInfo *index, - Path *basepath, List *unique_exprs) + Path *basepath, int prefix) { IndexPath *pathnode = makeNode(IndexPath); int numDistinctRows; - int distinctPrefixKeys; ListCell *lc; List *exprs = NIL; - - distinctPrefixKeys = list_length(unique_exprs); - Assert(IsA(basepath, IndexPath)); /* We don't want to modify basepath, so make a copy. */ memcpy(pathnode, basepath, sizeof(IndexPath)); - /* - * Normally we can think about distinctPrefixKeys as just - * a number of distinct keys. But if lets say we have a - * distinct key a, and the index contains b, a in exactly - * this order. In such situation we need to use position - * of a in the index as distinctPrefixKeys, otherwise skip - * will happen only by the first column. - */ - foreach(lc, unique_exprs) - { - Expr *unique_expr = (Expr *) lfirst(lc); - Var *var = (Var *) unique_expr; - - exprs = lappend(exprs, unique_expr); - - for (int i = 0; i < index->ncolumns; i++) - { - if (index->indexkeys[i] == var->varattno) - { - distinctPrefixKeys = Max(i + 1, distinctPrefixKeys); - break; - } - } - } - - Assert(distinctPrefixKeys > 0); - pathnode->indexskipprefix = distinctPrefixKeys; + Assert(prefix > 0); + pathnode->indexskipprefix = prefix; + pathnode->path.uniquekeys = root->query_uniquekeys; numDistinctRows = estimate_num_groups(root, exprs, pathnode->path.rows, diff --git a/src/include/optimizer/pathnode.h b/src/include/optimizer/pathnode.h index 705deaf7bd..8ec1780a56 100644 --- a/src/include/optimizer/pathnode.h +++ b/src/include/optimizer/pathnode.h @@ -210,7 +210,7 @@ extern UpperUniquePath *create_upper_unique_path(PlannerInfo *root, extern IndexPath *create_skipscan_unique_path(PlannerInfo *root, IndexOptInfo *index, Path *subpath, - List *unique_exprs); + int prefix); extern AggPath *create_agg_path(PlannerInfo *root, RelOptInfo *rel, Path *subpath, diff --git a/src/include/optimizer/paths.h b/src/include/optimizer/paths.h index 0cb8030e33..f934f0011a 100644 --- a/src/include/optimizer/paths.h +++ b/src/include/optimizer/paths.h @@ -198,6 +198,10 @@ extern Path *get_cheapest_fractional_path_for_pathkeys(List *paths, Relids required_outer, double fraction); extern Path *get_cheapest_parallel_safe_total_inner(List *paths); +extern int find_index_prefix_for_pathkey(PlannerInfo *root, + IndexOptInfo *index, + ScanDirection scandir, + PathKey *pathkey); extern List *build_index_pathkeys(PlannerInfo *root, IndexOptInfo *index, ScanDirection scandir); extern List *build_partition_pathkeys(PlannerInfo *root, RelOptInfo *partrel, diff --git a/src/test/regress/expected/sysviews.out b/src/test/regress/expected/sysviews.out index 4da9594925..e64e20a8cb 100644 --- a/src/test/regress/expected/sysviews.out +++ b/src/test/regress/expected/sysviews.out @@ -91,7 +91,7 @@ select name, setting from pg_settings where name like 'enable%'; enable_seqscan | on enable_sort | on enable_tidscan | on -(20 rows) +(19 rows) -- Test that the pg_timezone_names and pg_timezone_abbrevs views are -- more-or-less working. We can't test their contents in any great detail -- 2.25.0