public inbox for [email protected]
help / color / mirror / Atom feedFrom: Tomas Vondra <[email protected]>
Subject: [PATCH 4/8] fix
Date: Thu, 19 Mar 2020 16:55:28 +0100
---
src/backend/commands/explain.c | 6 ++--
src/backend/executor/execProcnode.c | 11 ++++----
src/backend/optimizer/plan/planner.c | 41 +++++++++++++++++-----------
src/backend/utils/sort/tuplesort.c | 10 +++++--
src/include/nodes/plannodes.h | 1 -
5 files changed, 42 insertions(+), 27 deletions(-)
diff --git a/src/backend/commands/explain.c b/src/backend/commands/explain.c
index dd4600a214..0256dd42f1 100644
--- a/src/backend/commands/explain.c
+++ b/src/backend/commands/explain.c
@@ -2767,7 +2767,7 @@ show_incremental_sort_group_info(IncrementalSortGroupInfo *groupInfo,
foreach(methodCell, groupInfo->sortMethods)
{
- const *sortMethodName = tuplesort_method_name(methodCell->int_value);
+ const char *sortMethodName = tuplesort_method_name(methodCell->int_value);
methodNames = lappend(methodNames, sortMethodName);
}
@@ -2776,7 +2776,7 @@ show_incremental_sort_group_info(IncrementalSortGroupInfo *groupInfo,
if (groupInfo->maxMemorySpaceUsed > 0)
{
long avgSpace = groupInfo->totalMemorySpaceUsed / groupInfo->groupCount;
- const *spaceTypeName;
+ const char *spaceTypeName;
ExplainPropertyInteger("Average Sort Space Used", "kB", avgSpace, es);
ExplainPropertyInteger("Maximum Sort Space Used", "kB",
@@ -2787,7 +2787,7 @@ show_incremental_sort_group_info(IncrementalSortGroupInfo *groupInfo,
if (groupInfo->maxDiskSpaceUsed > 0)
{
long avgSpace = groupInfo->totalDiskSpaceUsed / groupInfo->groupCount;
- const *spaceTypeName;
+ const char *spaceTypeName;
ExplainPropertyInteger("Average Sort Space Used", "kB", avgSpace, es);
ExplainPropertyInteger("Maximum Sort Space Used", "kB",
diff --git a/src/backend/executor/execProcnode.c b/src/backend/executor/execProcnode.c
index d15a86a706..5662e7d742 100644
--- a/src/backend/executor/execProcnode.c
+++ b/src/backend/executor/execProcnode.c
@@ -852,12 +852,13 @@ ExecSetTupleBound(int64 tuples_needed, PlanState *child_node)
else if (IsA(child_node, IncrementalSortState))
{
/*
- * If it is a Sort node, notify it that it can use bounded sort.
+ * If it is an IncrementalSort node, notify it that it can use bounded
+ * sort.
*
- * Note: it is the responsibility of nodeSort.c to react properly to
- * changes of these parameters. If we ever redesign this, it'd be a
- * good idea to integrate this signaling with the parameter-change
- * mechanism.
+ * Note: it is the responsibility of nodeIncrementalSort.c to react
+ * properly to changes of these parameters. If we ever redesign this,
+ * it'd be a good idea to integrate this signaling with the
+ * parameter-change mechanism.
*/
IncrementalSortState *sortState = (IncrementalSortState *) child_node;
diff --git a/src/backend/optimizer/plan/planner.c b/src/backend/optimizer/plan/planner.c
index b2f4aaadb5..0e01cf8cb1 100644
--- a/src/backend/optimizer/plan/planner.c
+++ b/src/backend/optimizer/plan/planner.c
@@ -4869,7 +4869,7 @@ create_distinct_paths(PlannerInfo *root,
else
{
Size hashentrysize = hash_agg_entry_size(
- 0, cheapest_input_path->pathtarget->width, 0);
+ 0, cheapest_input_path->pathtarget->width, 0);
/* Allow hashing only if hashtable is predicted to fit in work_mem */
allow_hash = (hashentrysize * numDistinctRows <= work_mem * 1024L);
@@ -4932,6 +4932,9 @@ create_distinct_paths(PlannerInfo *root,
* target: the output tlist the result Paths must emit
* limit_tuples: estimated bound on the number of output tuples,
* or -1 if no LIMIT or couldn't estimate
+ *
+ * XXX This only looks at sort_pathkeys. I wonder if it needs to look at the
+ * other pathkeys (grouping, ...) like generate_useful_gather_paths.
*/
static RelOptInfo *
create_ordered_paths(PlannerInfo *root,
@@ -5002,23 +5005,29 @@ create_ordered_paths(PlannerInfo *root,
add_path(ordered_rel, sorted_path);
}
- if (enable_incrementalsort && presorted_keys > 0)
- {
- /* Also consider incremental sort. */
- sorted_path = (Path *) create_incremental_sort_path(root,
- ordered_rel,
- input_path,
- root->sort_pathkeys,
- presorted_keys,
- limit_tuples);
- /* Add projection step if needed */
- if (sorted_path->pathtarget != target)
- sorted_path = apply_projection_to_path(root, ordered_rel,
- sorted_path, target);
+ /* With incremental sort disabled, don't build those paths. */
+ if (!enable_incrementalsort)
+ continue;
- add_path(ordered_rel, sorted_path);
- }
+ /* Likewise, if the path can't be used for incremental sort. */
+ if (!presorted_keys)
+ continue;
+
+ /* Also consider incremental sort. */
+ sorted_path = (Path *) create_incremental_sort_path(root,
+ ordered_rel,
+ input_path,
+ root->sort_pathkeys,
+ presorted_keys,
+ limit_tuples);
+
+ /* Add projection step if needed */
+ if (sorted_path->pathtarget != target)
+ sorted_path = apply_projection_to_path(root, ordered_rel,
+ sorted_path, target);
+
+ add_path(ordered_rel, sorted_path);
}
}
diff --git a/src/backend/utils/sort/tuplesort.c b/src/backend/utils/sort/tuplesort.c
index 583551d197..77c15ebd78 100644
--- a/src/backend/utils/sort/tuplesort.c
+++ b/src/backend/utils/sort/tuplesort.c
@@ -256,8 +256,8 @@ struct Tuplesortstate
bool isMaxSpaceDisk; /* true when maxSpace is value for on-disk
* space, false when it's value for in-memory
* space */
- TupSortStatus maxSpaceStatus; /* sort status when maxSpace was reached */
- MemoryContext maincontext; /* memory context for tuple sort metadata that
+ TupSortStatus maxSpaceStatus; /* sort status when maxSpace was reached */
+ MemoryContext maincontext; /* memory context for tuple sort metadata that
* persists across multiple batches */
MemoryContext sortcontext; /* memory context holding most sort data */
MemoryContext tuplecontext; /* sub-context of sortcontext for tuple data */
@@ -803,6 +803,9 @@ tuplesort_begin_common(int workMem, SortCoordinate coordinate,
return state;
}
+/*
+ * XXX Missing comment.
+ */
static void
tuplesort_begin_batch(Tuplesortstate *state)
{
@@ -1288,6 +1291,9 @@ tuplesort_set_bound(Tuplesortstate *state, int64 bound)
state->sortKeys->abbrev_full_comparator = NULL;
}
+/*
+ * XXX Missing comment.
+ */
bool
tuplesort_used_bound(Tuplesortstate *state)
{
diff --git a/src/include/nodes/plannodes.h b/src/include/nodes/plannodes.h
index fe4046b64b..136d794219 100644
--- a/src/include/nodes/plannodes.h
+++ b/src/include/nodes/plannodes.h
@@ -774,7 +774,6 @@ typedef struct Sort
bool *nullsFirst; /* NULLS FIRST/LAST directions */
} Sort;
-
/* ----------------
* incremental sort node
* ----------------
--
2.21.1
--dfcjsgdukgytabqd
Content-Type: text/plain; charset=us-ascii
Content-Disposition: attachment;
filename*0=v39-0005-Consider-incremental-sort-paths-in-additional-places;
filename*1=".patch"
view thread (10+ messages) latest in thread
reply
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: [email protected]
Cc: [email protected]
Subject: Re: [PATCH 4/8] fix
In-Reply-To: <no-message-id-1882714@localhost>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox