diff --git a/src/backend/optimizer/path/allpaths.c b/src/backend/optimizer/path/allpaths.c index 32bf734820..eea41fafe3 100644 --- a/src/backend/optimizer/path/allpaths.c +++ b/src/backend/optimizer/path/allpaths.c @@ -2727,63 +2727,6 @@ generate_gather_paths(PlannerInfo *root, RelOptInfo *rel, bool override_rows) } } -/* - * get_useful_pathkeys_for_relation - * Determine which orderings of a relation might be useful. - * - * Getting data in sorted order can be useful either because the requested - * order matches the final output ordering for the overall query we're - * planning, or because it enables an efficient merge join. Here, we try - * to figure out which pathkeys to consider. - * - * This allows us to do incremental sort on top of an index scan under a gather - * merge node, i.e. parallelized. - * - * XXX At the moment this can only ever return a list with a single element, - * because it looks at query_pathkeys only. So we might return the pathkeys - * directly, but it seems plausible we'll want to consider other orderings - * in the future. - */ -static List * -get_useful_pathkeys_for_relation(PlannerInfo *root, RelOptInfo *rel) -{ - List *useful_pathkeys_list = NIL; - ListCell *lc; - - /* - * Considering query_pathkeys is always worth it, because it might allow us - * to avoid a total sort when we have a partially presorted path available. - */ - if (root->query_pathkeys) - { - bool query_pathkeys_ok = true; - - foreach(lc, root->query_pathkeys) - { - PathKey *pathkey = (PathKey *) lfirst(lc); - EquivalenceClass *pathkey_ec = pathkey->pk_eclass; - Expr *em_expr; - - /* - * We can't use incremental sort for pathkeys containing volatile - * expressions. We could walk the exppression itself, but checking - * ec_has_volatile here saves some cycles. - */ - if (pathkey_ec->ec_has_volatile || - !(em_expr = find_em_expr_for_rel(pathkey_ec, rel))) - { - query_pathkeys_ok = false; - break; - } - } - - if (query_pathkeys_ok) - useful_pathkeys_list = list_make1(list_copy(root->query_pathkeys)); - } - - return useful_pathkeys_list; -} - /* * generate_useful_gather_paths * Generate parallel access paths for a relation by pushing a Gather or @@ -2800,7 +2743,6 @@ generate_useful_gather_paths(PlannerInfo *root, RelOptInfo *rel, bool override_r ListCell *lc; double rows; double *rowsp = NULL; - List *useful_pathkeys_list = NIL; Path *cheapest_partial_path = NULL; /* If there are no partial paths, there's nothing to do here. */ @@ -2818,9 +2760,6 @@ generate_useful_gather_paths(PlannerInfo *root, RelOptInfo *rel, bool override_r if (!enable_incrementalsort) return; - /* consider incremental sort for interesting orderings */ - useful_pathkeys_list = get_useful_pathkeys_for_relation(root, rel); - /* used for explicit (full) sort paths */ cheapest_partial_path = linitial(rel->partial_pathlist); @@ -2829,104 +2768,125 @@ generate_useful_gather_paths(PlannerInfo *root, RelOptInfo *rel, bool override_r * * XXX I wonder if we need to consider adding a projection here, as * create_ordered_paths does. + * + * XXX I wonder if it'd be worth verifying that at least some prefix + * of root->query_pathkeys has eclass members for this rel before + * looping through the subpaths. If, for example, the first pathkey + * isn't in this rel, then we know this loop won't ever be useful. */ - foreach(lc, useful_pathkeys_list) + foreach(lc, rel->partial_pathlist) { - List *useful_pathkeys = lfirst(lc); - ListCell *lc2; bool is_sorted; int presorted_keys; + Path *subpath = (Path *) lfirst(lc); + GatherMergePath *path; + List *useful_pathkeys = NIL; - foreach(lc2, rel->partial_pathlist) - { - Path *subpath = (Path *) lfirst(lc2); - GatherMergePath *path; - - /* path has no ordering at all, can't use incremental sort */ - if (subpath->pathkeys == NIL) - continue; + /* path has no ordering at all, can't use incremental sort */ + if (subpath->pathkeys == NIL) + continue; - is_sorted = pathkeys_common_contained_in(useful_pathkeys, - subpath->pathkeys, - &presorted_keys); + /* consider incremental sort for interesting orderings */ + useful_pathkeys = truncate_useless_pathkeys(root, rel, subpath->pathkeys); - /* - * When the partial path is already sorted, we can just add a gather - * merge on top, and we're done - no point in adding explicit sort. - * - * XXX Can't we skip this (maybe only for the cheapest partial path) - * when the path is already sorted? Then it's likely duplicate with - * the path created by generate_gather_paths. - */ - if (is_sorted) - { - path = create_gather_merge_path(root, rel, subpath, rel->reltarget, - subpath->pathkeys, NULL, rowsp); + /* + * Getting data in sorted order can be useful either because the requested + * order matches the final output ordering for the overall query we're + * planning, or because it enables an efficient merge join. Here, we try + * to figure out which pathkeys to consider. + * + * This allows us to do incremental sort on top of an index scan under a gather + * merge node, i.e. parallelized. + * + * XXX At the moment this can only ever return a list with a single element, + * because it looks at query_pathkeys only. So we might return the pathkeys + * directly, but it seems plausible we'll want to consider other orderings + * in the future. + * + * XXX Should this just be root->query_pathkeys == useful_pathkeys + * (or length of the two are equal) given our using truncate_useless_pathkeys + * above? + */ + is_sorted = pathkeys_common_contained_in(useful_pathkeys, + subpath->pathkeys, + &presorted_keys); - add_path(rel, &path->path); - continue; - } + /* + * When the partial path is already sorted, we can just add a gather + * merge on top, and we're done - no point in adding explicit sort. + * + * XXX Can't we skip this (maybe only for the cheapest partial path) + * when the path is already sorted? Then it's likely duplicate with + * the path created by generate_gather_paths. + */ + if (is_sorted) + { + path = create_gather_merge_path(root, rel, subpath, rel->reltarget, + subpath->pathkeys, NULL, rowsp); - Assert(!is_sorted); + add_path(rel, &path->path); + continue; + } - /* - * Consider regular sort for the cheapest partial path (for each - * useful pathkeys). We know the path is not sorted, because we'd - * not get here otherwise. - * - * XXX This is not redundant with the gather merge path created in - * generate_gather_paths, because that merely preserves ordering of - * the cheapest partial path, while here we add an explicit sort to - * get match the useful ordering. - */ - if (cheapest_partial_path == subpath) - { - Path *tmp; + Assert(!is_sorted); - tmp = (Path *) create_sort_path(root, - rel, - subpath, - useful_pathkeys, - -1.0); + /* + * Consider regular sort for the cheapest partial path (for each + * useful pathkeys). We know the path is not sorted, because we'd + * not get here otherwise. + * + * XXX This is not redundant with the gather merge path created in + * generate_gather_paths, because that merely preserves ordering of + * the cheapest partial path, while here we add an explicit sort to + * get match the useful ordering. + */ + if (cheapest_partial_path == subpath) + { + Path *tmp; - rows = tmp->rows * tmp->parallel_workers; + tmp = (Path *) create_sort_path(root, + rel, + subpath, + useful_pathkeys, + -1.0); - path = create_gather_merge_path(root, rel, - tmp, - rel->reltarget, - tmp->pathkeys, - NULL, - rowsp); + rows = tmp->rows * tmp->parallel_workers; - add_path(rel, &path->path); + path = create_gather_merge_path(root, rel, + tmp, + rel->reltarget, + tmp->pathkeys, + NULL, + rowsp); - /* Fall through */ - } + add_path(rel, &path->path); - /* - * Consider incremental sort, but only when the subpath is already - * partially sorted on a pathkey prefix. - */ - if (presorted_keys > 0) - { - Path *tmp; + /* Fall through */ + } - tmp = (Path *) create_incremental_sort_path(root, - rel, - subpath, - useful_pathkeys, - presorted_keys, - -1); - - path = create_gather_merge_path(root, rel, - tmp, - rel->reltarget, - tmp->pathkeys, - NULL, - rowsp); - - add_path(rel, &path->path); - } + /* + * Consider incremental sort, but only when the subpath is already + * partially sorted on a pathkey prefix. + */ + if (presorted_keys > 0) + { + Path *tmp; + + tmp = (Path *) create_incremental_sort_path(root, + rel, + subpath, + useful_pathkeys, + presorted_keys, + -1); + + path = create_gather_merge_path(root, rel, + tmp, + rel->reltarget, + tmp->pathkeys, + NULL, + rowsp); + + add_path(rel, &path->path); } } }