pg.ddx.io pgsql-hackers@postgresql.org mailing list archive
help / color / mirror / Atom feedsuggest to rename enable_incrementalsort
14+ messages / 9 participants
[nested] [flat]
Commits from this thread:
2020-07-05 94e454cddf e61225ffab Rename enable_incrementalsort for clarity
* suggest to rename enable_incrementalsort
@ 2020-06-21 06:26 Peter Eisentraut <peter.eisentraut@2ndquadrant.com>
2020-06-21 07:05 ` Re: suggest to rename enable_incrementalsort Julien Rouhaud <rjuju123@gmail.com>
0 siblings, 1 reply; 14+ messages in thread
From: Peter Eisentraut @ 2020-06-21 06:26 UTC (permalink / raw)
To: pgsql-hackers
I suggest to rename enable_incrementalsort to enable_incremental_sort.
This is obviously more readable and also how we have named recently
added multiword planner parameters.
See attached patch.
--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
Attachments:
0001-Rename-to-enable_incremental_sort-from-enable_increm.patch (text/plain, 11.8K) download
From 4c55bf802f42df5366d9c734bb9527d57de57de4 Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <peter@eisentraut.org>
Date: Sun, 21 Jun 2020 08:21:53 +0200
Subject: [PATCH] Rename to enable_incremental_sort from enable_incrementalsort
---
doc/src/sgml/config.sgml | 6 +++---
doc/src/sgml/release-13.sgml | 4 ++--
src/backend/optimizer/path/allpaths.c | 2 +-
src/backend/optimizer/path/costsize.c | 2 +-
src/backend/optimizer/plan/planner.c | 14 +++++++-------
src/backend/utils/misc/guc.c | 4 ++--
src/backend/utils/misc/postgresql.conf.sample | 2 +-
src/include/optimizer/cost.h | 2 +-
src/test/regress/expected/incremental_sort.out | 4 ++--
src/test/regress/expected/partition_aggregate.out | 2 +-
src/test/regress/expected/sysviews.out | 2 +-
src/test/regress/sql/incremental_sort.sql | 4 ++--
src/test/regress/sql/partition_aggregate.sql | 2 +-
13 files changed, 25 insertions(+), 25 deletions(-)
diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml
index c6294df936..4c6daac86e 100644
--- a/doc/src/sgml/config.sgml
+++ b/doc/src/sgml/config.sgml
@@ -4591,10 +4591,10 @@ <title>Planner Method Configuration</title>
</listitem>
</varlistentry>
- <varlistentry id="guc-enable-incrementalsort" xreflabel="enable_incrementalsort">
- <term><varname>enable_incrementalsort</varname> (<type>boolean</type>)
+ <varlistentry id="guc-enable-incremental-sort" xreflabel="enable_incremental_sort">
+ <term><varname>enable_incremental_sort</varname> (<type>boolean</type>)
<indexterm>
- <primary><varname>enable_incrementalsort</varname> configuration parameter</primary>
+ <primary><varname>enable_incremental_sort</varname> configuration parameter</primary>
</indexterm>
</term>
<listitem>
diff --git a/doc/src/sgml/release-13.sgml b/doc/src/sgml/release-13.sgml
index 401c557373..085766b8c4 100644
--- a/doc/src/sgml/release-13.sgml
+++ b/doc/src/sgml/release-13.sgml
@@ -588,7 +588,7 @@ <title>General Performance</title>
-->
<para>
- Implement <link linkend="guc-enable-incrementalsort">incremental
+ Implement <link linkend="guc-enable-incremental-sort">incremental
sorting</link> (James Coleman, Alexander Korotkov, Tomas Vondra)
</para>
@@ -596,7 +596,7 @@ <title>General Performance</title>
If a result is already sorted by several leading keys, this
allows for batch sorting of additional trailing keys because the
previous keys are already equal. This is controlled by <xref
- linkend="guc-enable-incrementalsort"/>.
+ linkend="guc-enable-incremental-sort"/>.
</para>
</listitem>
diff --git a/src/backend/optimizer/path/allpaths.c b/src/backend/optimizer/path/allpaths.c
index d984da25d7..81896de936 100644
--- a/src/backend/optimizer/path/allpaths.c
+++ b/src/backend/optimizer/path/allpaths.c
@@ -2912,7 +2912,7 @@ generate_useful_gather_paths(PlannerInfo *root, RelOptInfo *rel, bool override_r
* Consider incremental sort, but only when the subpath is already
* partially sorted on a pathkey prefix.
*/
- if (enable_incrementalsort && presorted_keys > 0)
+ if (enable_incremental_sort && presorted_keys > 0)
{
Path *tmp;
diff --git a/src/backend/optimizer/path/costsize.c b/src/backend/optimizer/path/costsize.c
index 4ff3c7a2fd..87c9b49ce1 100644
--- a/src/backend/optimizer/path/costsize.c
+++ b/src/backend/optimizer/path/costsize.c
@@ -128,7 +128,7 @@ bool enable_indexonlyscan = true;
bool enable_bitmapscan = true;
bool enable_tidscan = true;
bool enable_sort = true;
-bool enable_incrementalsort = true;
+bool enable_incremental_sort = true;
bool enable_hashagg = true;
bool hashagg_avoid_disk_plan = true;
bool enable_nestloop = true;
diff --git a/src/backend/optimizer/plan/planner.c b/src/backend/optimizer/plan/planner.c
index 4131019fc9..14f3fd44e3 100644
--- a/src/backend/optimizer/plan/planner.c
+++ b/src/backend/optimizer/plan/planner.c
@@ -5014,7 +5014,7 @@ create_ordered_paths(PlannerInfo *root,
* presorted the path is. Additionally incremental sort may enable
* a cheaper startup path to win out despite higher total cost.
*/
- if (!enable_incrementalsort)
+ if (!enable_incremental_sort)
continue;
/* Likewise, if the path can't be used for incremental sort. */
@@ -5095,7 +5095,7 @@ create_ordered_paths(PlannerInfo *root,
* sort_pathkeys because then we can't possibly have a presorted
* prefix of the list without having the list be fully sorted.
*/
- if (enable_incrementalsort && list_length(root->sort_pathkeys) > 1)
+ if (enable_incremental_sort && list_length(root->sort_pathkeys) > 1)
{
ListCell *lc;
@@ -6572,7 +6572,7 @@ add_paths_to_grouping_rel(PlannerInfo *root, RelOptInfo *input_rel,
* when the path is not already sorted and when incremental sort
* is enabled.
*/
- if (is_sorted || !enable_incrementalsort)
+ if (is_sorted || !enable_incremental_sort)
continue;
/* Restore the input path (we might have added Sort on top). */
@@ -6699,7 +6699,7 @@ add_paths_to_grouping_rel(PlannerInfo *root, RelOptInfo *input_rel,
* when the path is not already sorted and when incremental
* sort is enabled.
*/
- if (is_sorted || !enable_incrementalsort)
+ if (is_sorted || !enable_incremental_sort)
continue;
/* Restore the input path (we might have added Sort on top). */
@@ -7022,7 +7022,7 @@ create_partial_grouping_paths(PlannerInfo *root,
* group_pathkeys because then we can't possibly have a presorted
* prefix of the list without having the list be fully sorted.
*/
- if (enable_incrementalsort && list_length(root->group_pathkeys) > 1)
+ if (enable_incremental_sort && list_length(root->group_pathkeys) > 1)
{
foreach(lc, input_rel->pathlist)
{
@@ -7125,7 +7125,7 @@ create_partial_grouping_paths(PlannerInfo *root,
* when the path is not already sorted and when incremental sort
* is enabled.
*/
- if (is_sorted || !enable_incrementalsort)
+ if (is_sorted || !enable_incremental_sort)
continue;
/* Restore the input path (we might have added Sort on top). */
@@ -7304,7 +7304,7 @@ gather_grouping_paths(PlannerInfo *root, RelOptInfo *rel)
* group_pathkeys because then we can't possibly have a presorted prefix
* of the list without having the list be fully sorted.
*/
- if (!enable_incrementalsort || list_length(root->group_pathkeys) == 1)
+ if (!enable_incremental_sort || list_length(root->group_pathkeys) == 1)
return;
/* also consider incremental sort on partial paths, if enabled */
diff --git a/src/backend/utils/misc/guc.c b/src/backend/utils/misc/guc.c
index fe7e2f8b91..9f5e223920 100644
--- a/src/backend/utils/misc/guc.c
+++ b/src/backend/utils/misc/guc.c
@@ -992,11 +992,11 @@ static struct config_bool ConfigureNamesBool[] =
NULL, NULL, NULL
},
{
- {"enable_incrementalsort", PGC_USERSET, QUERY_TUNING_METHOD,
+ {"enable_incremental_sort", PGC_USERSET, QUERY_TUNING_METHOD,
gettext_noop("Enables the planner's use of incremental sort steps."),
NULL
},
- &enable_incrementalsort,
+ &enable_incremental_sort,
true,
NULL, NULL, NULL
},
diff --git a/src/backend/utils/misc/postgresql.conf.sample b/src/backend/utils/misc/postgresql.conf.sample
index ac02bd0c00..bd4cb8144b 100644
--- a/src/backend/utils/misc/postgresql.conf.sample
+++ b/src/backend/utils/misc/postgresql.conf.sample
@@ -361,7 +361,7 @@
#enable_parallel_append = on
#enable_seqscan = on
#enable_sort = on
-#enable_incrementalsort = on
+#enable_incremental_sort = on
#enable_tidscan = on
#enable_partitionwise_join = off
#enable_partitionwise_aggregate = off
diff --git a/src/include/optimizer/cost.h b/src/include/optimizer/cost.h
index 92e70ec0d9..613db8eab6 100644
--- a/src/include/optimizer/cost.h
+++ b/src/include/optimizer/cost.h
@@ -53,7 +53,7 @@ extern PGDLLIMPORT bool enable_indexonlyscan;
extern PGDLLIMPORT bool enable_bitmapscan;
extern PGDLLIMPORT bool enable_tidscan;
extern PGDLLIMPORT bool enable_sort;
-extern PGDLLIMPORT bool enable_incrementalsort;
+extern PGDLLIMPORT bool enable_incremental_sort;
extern PGDLLIMPORT bool enable_hashagg;
extern PGDLLIMPORT bool hashagg_avoid_disk_plan;
extern PGDLLIMPORT bool enable_nestloop;
diff --git a/src/test/regress/expected/incremental_sort.out b/src/test/regress/expected/incremental_sort.out
index 53accd0df9..e376ea1276 100644
--- a/src/test/regress/expected/incremental_sort.out
+++ b/src/test/regress/expected/incremental_sort.out
@@ -1414,7 +1414,7 @@ create table t (a int, b int, c int);
insert into t select mod(i,10),mod(i,10),i from generate_series(1,10000) s(i);
create index on t (a);
analyze t;
-set enable_incrementalsort = off;
+set enable_incremental_sort = off;
explain (costs off) select a,b,sum(c) from t group by 1,2 order by 1,2,3 limit 1;
QUERY PLAN
------------------------------------------------------
@@ -1430,7 +1430,7 @@ explain (costs off) select a,b,sum(c) from t group by 1,2 order by 1,2,3 limit 1
-> Parallel Seq Scan on t
(10 rows)
-set enable_incrementalsort = on;
+set enable_incremental_sort = on;
explain (costs off) select a,b,sum(c) from t group by 1,2 order by 1,2,3 limit 1;
QUERY PLAN
----------------------------------------------------------------------
diff --git a/src/test/regress/expected/partition_aggregate.out b/src/test/regress/expected/partition_aggregate.out
index c36970575f..45c698daf4 100644
--- a/src/test/regress/expected/partition_aggregate.out
+++ b/src/test/regress/expected/partition_aggregate.out
@@ -12,7 +12,7 @@ SET enable_partitionwise_join TO true;
-- Disable parallel plans.
SET max_parallel_workers_per_gather TO 0;
-- Disable incremental sort, which can influence selected plans due to fuzz factor.
-SET enable_incrementalsort TO off;
+SET enable_incremental_sort TO off;
--
-- Tests for list partitioned tables.
--
diff --git a/src/test/regress/expected/sysviews.out b/src/test/regress/expected/sysviews.out
index 01b7786f01..06c4c3e476 100644
--- a/src/test/regress/expected/sysviews.out
+++ b/src/test/regress/expected/sysviews.out
@@ -76,7 +76,7 @@ select name, setting from pg_settings where name like 'enable%';
enable_gathermerge | on
enable_hashagg | on
enable_hashjoin | on
- enable_incrementalsort | on
+ enable_incremental_sort | on
enable_indexonlyscan | on
enable_indexscan | on
enable_material | on
diff --git a/src/test/regress/sql/incremental_sort.sql b/src/test/regress/sql/incremental_sort.sql
index 373e62ac13..9c040c90e6 100644
--- a/src/test/regress/sql/incremental_sort.sql
+++ b/src/test/regress/sql/incremental_sort.sql
@@ -210,10 +210,10 @@
create index on t (a);
analyze t;
-set enable_incrementalsort = off;
+set enable_incremental_sort = off;
explain (costs off) select a,b,sum(c) from t group by 1,2 order by 1,2,3 limit 1;
-set enable_incrementalsort = on;
+set enable_incremental_sort = on;
explain (costs off) select a,b,sum(c) from t group by 1,2 order by 1,2,3 limit 1;
-- Incremental sort vs. set operations with varno 0
diff --git a/src/test/regress/sql/partition_aggregate.sql b/src/test/regress/sql/partition_aggregate.sql
index 178f2079fa..117f65ecb4 100644
--- a/src/test/regress/sql/partition_aggregate.sql
+++ b/src/test/regress/sql/partition_aggregate.sql
@@ -13,7 +13,7 @@
-- Disable parallel plans.
SET max_parallel_workers_per_gather TO 0;
-- Disable incremental sort, which can influence selected plans due to fuzz factor.
-SET enable_incrementalsort TO off;
+SET enable_incremental_sort TO off;
--
-- Tests for list partitioned tables.
--
2.27.0
* Re: suggest to rename enable_incrementalsort
2020-06-21 06:26 suggest to rename enable_incrementalsort Peter Eisentraut <peter.eisentraut@2ndquadrant.com>
@ 2020-06-21 07:05 ` Julien Rouhaud <rjuju123@gmail.com>
2020-06-21 11:21 ` Re: suggest to rename enable_incrementalsort Tomas Vondra <tomas.vondra@2ndquadrant.com>
0 siblings, 1 reply; 14+ messages in thread
From: Julien Rouhaud @ 2020-06-21 07:05 UTC (permalink / raw)
To: Peter Eisentraut <peter.eisentraut@2ndquadrant.com>; +Cc: pgsql-hackers
On Sun, Jun 21, 2020 at 8:26 AM Peter Eisentraut
<peter.eisentraut@2ndquadrant.com> wrote:
>
> I suggest to rename enable_incrementalsort to enable_incremental_sort.
> This is obviously more readable and also how we have named recently
> added multiword planner parameters.
>
> See attached patch.
+1, this is a way better name (and patch LGTM on REL_13_STABLE).
^ permalink raw reply [nested|flat] 14+ messages in thread
* Re: suggest to rename enable_incrementalsort
2020-06-21 06:26 suggest to rename enable_incrementalsort Peter Eisentraut <peter.eisentraut@2ndquadrant.com>
2020-06-21 07:05 ` Re: suggest to rename enable_incrementalsort Julien Rouhaud <rjuju123@gmail.com>
@ 2020-06-21 11:21 ` Tomas Vondra <tomas.vondra@2ndquadrant.com>
2020-06-21 23:18 ` Re: suggest to rename enable_incrementalsort David Rowley <dgrowleyml@gmail.com>
2020-06-22 14:16 ` Re: suggest to rename enable_incrementalsort Robert Haas <robertmhaas@gmail.com>
0 siblings, 2 replies; 14+ messages in thread
From: Tomas Vondra @ 2020-06-21 11:21 UTC (permalink / raw)
To: Julien Rouhaud <rjuju123@gmail.com>; +Cc: Peter Eisentraut <peter.eisentraut@2ndquadrant.com>; pgsql-hackers
On Sun, Jun 21, 2020 at 09:05:32AM +0200, Julien Rouhaud wrote:
>On Sun, Jun 21, 2020 at 8:26 AM Peter Eisentraut
><peter.eisentraut@2ndquadrant.com> wrote:
>>
>> I suggest to rename enable_incrementalsort to enable_incremental_sort.
>> This is obviously more readable and also how we have named recently
>> added multiword planner parameters.
>>
>> See attached patch.
>
>+1, this is a way better name (and patch LGTM on REL_13_STABLE).
>
The reason why I kept the single-word variant is consistency with other
GUCs that affect planning, like enable_indexscan, enable_hashjoin and
many others.
That being said, I'm not particularly attached this choice, so if you
think this is better I'm OK with it.
regards
--
Tomas Vondra http://www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
^ permalink raw reply [nested|flat] 14+ messages in thread
* Re: suggest to rename enable_incrementalsort
2020-06-21 06:26 suggest to rename enable_incrementalsort Peter Eisentraut <peter.eisentraut@2ndquadrant.com>
2020-06-21 07:05 ` Re: suggest to rename enable_incrementalsort Julien Rouhaud <rjuju123@gmail.com>
2020-06-21 11:21 ` Re: suggest to rename enable_incrementalsort Tomas Vondra <tomas.vondra@2ndquadrant.com>
@ 2020-06-21 23:18 ` David Rowley <dgrowleyml@gmail.com>
2020-06-22 11:55 ` Re: suggest to rename enable_incrementalsort Ashutosh Bapat <ashutosh.bapat.oss@gmail.com>
1 sibling, 1 reply; 14+ messages in thread
From: David Rowley @ 2020-06-21 23:18 UTC (permalink / raw)
To: Tomas Vondra <tomas.vondra@2ndquadrant.com>; +Cc: Julien Rouhaud <rjuju123@gmail.com>; Peter Eisentraut <peter.eisentraut@2ndquadrant.com>; pgsql-hackers
On Sun, 21 Jun 2020 at 23:22, Tomas Vondra <tomas.vondra@2ndquadrant.com> wrote:
>
> On Sun, Jun 21, 2020 at 09:05:32AM +0200, Julien Rouhaud wrote:
> >On Sun, Jun 21, 2020 at 8:26 AM Peter Eisentraut
> ><peter.eisentraut@2ndquadrant.com> wrote:
> >>
> >> I suggest to rename enable_incrementalsort to enable_incremental_sort.
> >> This is obviously more readable and also how we have named recently
> >> added multiword planner parameters.
> >>
> >> See attached patch.
> >
> >+1, this is a way better name (and patch LGTM on REL_13_STABLE).
> >
>
> The reason why I kept the single-word variant is consistency with other
> GUCs that affect planning, like enable_indexscan, enable_hashjoin and
> many others.
Looking at the other enable_* GUCs, for all the ones that aim to
disable a certain executor node type, with the exception of
enable_hashagg and enable_bitmapscan, they're all pretty consistent in
naming the GUC after the executor node's .c file:
enable_bitmapscan nodeBitmapHeapscan.c
enable_gathermerge nodeGatherMerge.c
enable_hashagg nodeAgg.c
enable_hashjoin nodeHashjoin.c
enable_incrementalsort nodeIncrementalSort.c
enable_indexonlyscan nodeIndexonlyscan.c
enable_indexscan nodeIndexscan.c
enable_material nodeMaterial.c
enable_mergejoin nodeMergejoin.c
enable_nestloop nodeNestloop.c
enable_parallel_append nodeAppend.c
enable_parallel_hash nodeHash.c
enable_partition_pruning
enable_partitionwise_aggregate
enable_partitionwise_join
enable_seqscan nodeSeqscan.c
enable_sort nodeSort.c
enable_tidscan nodeTidscan.c
enable_partition_pruning, enable_partitionwise_aggregate,
enable_partitionwise_join are the odd ones out here as they're not
really related to a specific node type.
Going by that, it does seem the current name for
enable_incrementalsort is consistent with the majority. Naming it
enable_incremental_sort looks like it would be more suited if the
feature had been added by overloading nodeSort.c. In that regard, it
would be similar to enable_parallel_append and enable_parallel_hash,
where the middle word becomes a modifier.
David
^ permalink raw reply [nested|flat] 14+ messages in thread
* Re: suggest to rename enable_incrementalsort
2020-06-21 06:26 suggest to rename enable_incrementalsort Peter Eisentraut <peter.eisentraut@2ndquadrant.com>
2020-06-21 07:05 ` Re: suggest to rename enable_incrementalsort Julien Rouhaud <rjuju123@gmail.com>
2020-06-21 11:21 ` Re: suggest to rename enable_incrementalsort Tomas Vondra <tomas.vondra@2ndquadrant.com>
2020-06-21 23:18 ` Re: suggest to rename enable_incrementalsort David Rowley <dgrowleyml@gmail.com>
@ 2020-06-22 11:55 ` Ashutosh Bapat <ashutosh.bapat.oss@gmail.com>
0 siblings, 0 replies; 14+ messages in thread
From: Ashutosh Bapat @ 2020-06-22 11:55 UTC (permalink / raw)
To: David Rowley <dgrowleyml@gmail.com>; Ashutosh Bapat <ashutosh.bapat@2ndquadrant.com>; +Cc: Tomas Vondra <tomas.vondra@2ndquadrant.com>; Julien Rouhaud <rjuju123@gmail.com>; Peter Eisentraut <peter.eisentraut@2ndquadrant.com>; pgsql-hackers
On Mon, Jun 22, 2020 at 4:48 AM David Rowley <dgrowleyml@gmail.com> wrote:
>
> On Sun, 21 Jun 2020 at 23:22, Tomas Vondra <tomas.vondra@2ndquadrant.com> wrote:
> >
> > On Sun, Jun 21, 2020 at 09:05:32AM +0200, Julien Rouhaud wrote:
> > >On Sun, Jun 21, 2020 at 8:26 AM Peter Eisentraut
> > ><peter.eisentraut@2ndquadrant.com> wrote:
> > >>
> > >> I suggest to rename enable_incrementalsort to enable_incremental_sort.
> > >> This is obviously more readable and also how we have named recently
> > >> added multiword planner parameters.
> > >>
> > >> See attached patch.
> > >
> > >+1, this is a way better name (and patch LGTM on REL_13_STABLE).
> > >
> >
> > The reason why I kept the single-word variant is consistency with other
> > GUCs that affect planning, like enable_indexscan, enable_hashjoin and
> > many others.
>
> Looking at the other enable_* GUCs, for all the ones that aim to
> disable a certain executor node type, with the exception of
> enable_hashagg and enable_bitmapscan, they're all pretty consistent in
> naming the GUC after the executor node's .c file:
>
> enable_bitmapscan nodeBitmapHeapscan.c
> enable_gathermerge nodeGatherMerge.c
> enable_hashagg nodeAgg.c
> enable_hashjoin nodeHashjoin.c
> enable_incrementalsort nodeIncrementalSort.c
> enable_indexonlyscan nodeIndexonlyscan.c
> enable_indexscan nodeIndexscan.c
> enable_material nodeMaterial.c
> enable_mergejoin nodeMergejoin.c
> enable_nestloop nodeNestloop.c
> enable_parallel_append nodeAppend.c
> enable_parallel_hash nodeHash.c
> enable_partition_pruning
> enable_partitionwise_aggregate
> enable_partitionwise_join
> enable_seqscan nodeSeqscan.c
> enable_sort nodeSort.c
> enable_tidscan nodeTidscan.c
>
> enable_partition_pruning, enable_partitionwise_aggregate,
> enable_partitionwise_join are the odd ones out here as they're not
> really related to a specific node type.
Thanks for the list. To me it's more of a question about readability
than consistency. enable_mergejoin, enable_hashjoin for example are
readable even without separating words merge_join or hash_join (many
times I have typed enable_hash_join and cursed :); but that was before
autocomplete was available). But enable_partitionwiseaggregate does
not look much different from enable_abracadabra :). Looking from that
angle, enable_incremental_sort is better than enable_incrementalsort.
We could have named enable_indexonlyscan as enable_index_only_scan for
better readability.
--
Best Wishes,
Ashutosh Bapat
^ permalink raw reply [nested|flat] 14+ messages in thread
* Re: suggest to rename enable_incrementalsort
2020-06-21 06:26 suggest to rename enable_incrementalsort Peter Eisentraut <peter.eisentraut@2ndquadrant.com>
2020-06-21 07:05 ` Re: suggest to rename enable_incrementalsort Julien Rouhaud <rjuju123@gmail.com>
2020-06-21 11:21 ` Re: suggest to rename enable_incrementalsort Tomas Vondra <tomas.vondra@2ndquadrant.com>
@ 2020-06-22 14:16 ` Robert Haas <robertmhaas@gmail.com>
2020-06-22 14:31 ` Re: suggest to rename enable_incrementalsort Tomas Vondra <tomas.vondra@2ndquadrant.com>
2020-06-22 14:41 ` Re: suggest to rename enable_incrementalsort Tom Lane <tgl@sss.pgh.pa.us>
1 sibling, 2 replies; 14+ messages in thread
From: Robert Haas @ 2020-06-22 14:16 UTC (permalink / raw)
To: Tomas Vondra <tomas.vondra@2ndquadrant.com>; +Cc: Julien Rouhaud <rjuju123@gmail.com>; Peter Eisentraut <peter.eisentraut@2ndquadrant.com>; pgsql-hackers
On Sun, Jun 21, 2020 at 7:22 AM Tomas Vondra
<tomas.vondra@2ndquadrant.com> wrote:
> The reason why I kept the single-word variant is consistency with other
> GUCs that affect planning, like enable_indexscan, enable_hashjoin and
> many others.
Right, so that makes sense, but from a larger point of view, how much
sense does it actually make? I mean, I get the argument from tradition
and from internal naming consistency, but from a user perspective, why
does it makes sense for there to be underscores between some of the
words and not others? I think it just feels random, like someone is
charging us $1 per underscore so we're economizing.
So I'm +1 for changing this, and I'd definitely be +1 for renaming the
others if they weren't released already, and at least +0.5 for it
anyhow. It's bad enough that our source code has names_like_this and
NamesLikeThis and namesLikeThis; when we also start adding
names_likethis and NamesLike_this and maybe NaMeS___LiKeTh_is, I kind
of lose my mind. And avoiding that sort of thing in user-facing stuff
seems even more important.
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
^ permalink raw reply [nested|flat] 14+ messages in thread
* Re: suggest to rename enable_incrementalsort
2020-06-21 06:26 suggest to rename enable_incrementalsort Peter Eisentraut <peter.eisentraut@2ndquadrant.com>
2020-06-21 07:05 ` Re: suggest to rename enable_incrementalsort Julien Rouhaud <rjuju123@gmail.com>
2020-06-21 11:21 ` Re: suggest to rename enable_incrementalsort Tomas Vondra <tomas.vondra@2ndquadrant.com>
2020-06-22 14:16 ` Re: suggest to rename enable_incrementalsort Robert Haas <robertmhaas@gmail.com>
@ 2020-06-22 14:31 ` Tomas Vondra <tomas.vondra@2ndquadrant.com>
2020-06-22 15:13 ` Re: suggest to rename enable_incrementalsort Robert Haas <robertmhaas@gmail.com>
1 sibling, 1 reply; 14+ messages in thread
From: Tomas Vondra @ 2020-06-22 14:31 UTC (permalink / raw)
To: Robert Haas <robertmhaas@gmail.com>; +Cc: Julien Rouhaud <rjuju123@gmail.com>; Peter Eisentraut <peter.eisentraut@2ndquadrant.com>; pgsql-hackers
On Mon, Jun 22, 2020 at 10:16:54AM -0400, Robert Haas wrote:
>On Sun, Jun 21, 2020 at 7:22 AM Tomas Vondra
><tomas.vondra@2ndquadrant.com> wrote:
>> The reason why I kept the single-word variant is consistency with other
>> GUCs that affect planning, like enable_indexscan, enable_hashjoin and
>> many others.
>
>Right, so that makes sense, but from a larger point of view, how much
>sense does it actually make? I mean, I get the argument from tradition
>and from internal naming consistency, but from a user perspective, why
>does it makes sense for there to be underscores between some of the
>words and not others? I think it just feels random, like someone is
>charging us $1 per underscore so we're economizing.
>
Sure. I'm not particularly attached to the current GUC, I've only tried
to explain that the naming was not entirely random. I agree having an
extra _ in the name would make it more readable.
>So I'm +1 for changing this, and I'd definitely be +1 for renaming the
>others if they weren't released already, and at least +0.5 for it
>anyhow. It's bad enough that our source code has names_like_this and
>NamesLikeThis and namesLikeThis; when we also start adding
>names_likethis and NamesLike_this and maybe NaMeS___LiKeTh_is, I kind
>of lose my mind. And avoiding that sort of thing in user-facing stuff
>seems even more important.
>
OK, challenge accepted. $100 to the first person who commits a patch
with a variable NaMeS___LiKeTh_is.
regards
--
Tomas Vondra http://www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
^ permalink raw reply [nested|flat] 14+ messages in thread
* Re: suggest to rename enable_incrementalsort
2020-06-21 06:26 suggest to rename enable_incrementalsort Peter Eisentraut <peter.eisentraut@2ndquadrant.com>
2020-06-21 07:05 ` Re: suggest to rename enable_incrementalsort Julien Rouhaud <rjuju123@gmail.com>
2020-06-21 11:21 ` Re: suggest to rename enable_incrementalsort Tomas Vondra <tomas.vondra@2ndquadrant.com>
2020-06-22 14:16 ` Re: suggest to rename enable_incrementalsort Robert Haas <robertmhaas@gmail.com>
2020-06-22 14:31 ` Re: suggest to rename enable_incrementalsort Tomas Vondra <tomas.vondra@2ndquadrant.com>
@ 2020-06-22 15:13 ` Robert Haas <robertmhaas@gmail.com>
0 siblings, 0 replies; 14+ messages in thread
From: Robert Haas @ 2020-06-22 15:13 UTC (permalink / raw)
To: Tomas Vondra <tomas.vondra@2ndquadrant.com>; +Cc: Julien Rouhaud <rjuju123@gmail.com>; Peter Eisentraut <peter.eisentraut@2ndquadrant.com>; pgsql-hackers
On Mon, Jun 22, 2020 at 10:31 AM Tomas Vondra
<tomas.vondra@2ndquadrant.com> wrote:
> OK, challenge accepted. $100 to the first person who commits a patch
> with a variable NaMeS___LiKeTh_is.
:-)
Well, that was hyperbole, but people have proposed some pretty wacky
schemes, and a few of those have ended up in the tree. For example we
have AtEOXact_PgStat and its close friend AtEOXact_on_commit_actions,
for instance, or out_gistxlogDelete, or
IncrementVarSublevelsUp_rtable, or convert_EXISTS_sublink_to_join. I
confess haven't managed to find any plausible examples of underscores
in the middle of a word yet, and we only have a handful of examples of
double-underscore and none with triple-underscore, but we've got
nearly every combination of lower-case words, upper-case words,
initial-capital words, underscores separating words or not, and words
abbreviated or not, and it's not hard to find cases where several
different styles are used in the same identifier. This isn't the end
of the world or anything, but I think we would be better off if we
tried to do less of it.
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
^ permalink raw reply [nested|flat] 14+ messages in thread
* Re: suggest to rename enable_incrementalsort
2020-06-21 06:26 suggest to rename enable_incrementalsort Peter Eisentraut <peter.eisentraut@2ndquadrant.com>
2020-06-21 07:05 ` Re: suggest to rename enable_incrementalsort Julien Rouhaud <rjuju123@gmail.com>
2020-06-21 11:21 ` Re: suggest to rename enable_incrementalsort Tomas Vondra <tomas.vondra@2ndquadrant.com>
2020-06-22 14:16 ` Re: suggest to rename enable_incrementalsort Robert Haas <robertmhaas@gmail.com>
@ 2020-06-22 14:41 ` Tom Lane <tgl@sss.pgh.pa.us>
2020-06-22 15:22 ` Re: suggest to rename enable_incrementalsort Bruce Momjian <bruce@momjian.us>
1 sibling, 1 reply; 14+ messages in thread
From: Tom Lane @ 2020-06-22 14:41 UTC (permalink / raw)
To: Robert Haas <robertmhaas@gmail.com>; +Cc: Tomas Vondra <tomas.vondra@2ndquadrant.com>; Julien Rouhaud <rjuju123@gmail.com>; Peter Eisentraut <peter.eisentraut@2ndquadrant.com>; pgsql-hackers
Robert Haas <robertmhaas@gmail.com> writes:
> On Sun, Jun 21, 2020 at 7:22 AM Tomas Vondra
> <tomas.vondra@2ndquadrant.com> wrote:
>> The reason why I kept the single-word variant is consistency with other
>> GUCs that affect planning, like enable_indexscan, enable_hashjoin and
>> many others.
> Right, so that makes sense, but from a larger point of view, how much
> sense does it actually make?
Maybe I'm just used to the names, but I find that things like
"enable_seqscan" and "enable_nestloop" are pretty readable.
Once they get longer, though, not so much. So I agree with
renaming enable_incrementalsort.
> So I'm +1 for changing this, and I'd definitely be +1 for renaming the
> others if they weren't released already, and at least +0.5 for it
> anyhow.
Nah. Those names are way too well entrenched. Besides which, if
we open them up for reconsideration, there's going to be a lot of
bikeshedding done. Should "enable_seqscan" become "enable_seq_scan",
or "enable_sequential_scan", or maybe "enable_scan_sequential"?
Why doesn't "enable_nestloop" contain the word "join"? Etc etc.
(I do have to wonder if maybe this one should be enable_sort_incremental.)
regards, tom lane
^ permalink raw reply [nested|flat] 14+ messages in thread
* Re: suggest to rename enable_incrementalsort
2020-06-21 06:26 suggest to rename enable_incrementalsort Peter Eisentraut <peter.eisentraut@2ndquadrant.com>
2020-06-21 07:05 ` Re: suggest to rename enable_incrementalsort Julien Rouhaud <rjuju123@gmail.com>
2020-06-21 11:21 ` Re: suggest to rename enable_incrementalsort Tomas Vondra <tomas.vondra@2ndquadrant.com>
2020-06-22 14:16 ` Re: suggest to rename enable_incrementalsort Robert Haas <robertmhaas@gmail.com>
2020-06-22 14:41 ` Re: suggest to rename enable_incrementalsort Tom Lane <tgl@sss.pgh.pa.us>
@ 2020-06-22 15:22 ` Bruce Momjian <bruce@momjian.us>
2020-06-22 16:16 ` Re: suggest to rename enable_incrementalsort Tom Lane <tgl@sss.pgh.pa.us>
2020-06-22 16:17 ` Re: suggest to rename enable_incrementalsort Robert Haas <robertmhaas@gmail.com>
0 siblings, 2 replies; 14+ messages in thread
From: Bruce Momjian @ 2020-06-22 15:22 UTC (permalink / raw)
To: Tom Lane <tgl@sss.pgh.pa.us>; +Cc: Robert Haas <robertmhaas@gmail.com>; Tomas Vondra <tomas.vondra@2ndquadrant.com>; Julien Rouhaud <rjuju123@gmail.com>; Peter Eisentraut <peter.eisentraut@2ndquadrant.com>; pgsql-hackers
On Mon, Jun 22, 2020 at 10:41:17AM -0400, Tom Lane wrote:
> Robert Haas <robertmhaas@gmail.com> writes:
> > On Sun, Jun 21, 2020 at 7:22 AM Tomas Vondra
> > <tomas.vondra@2ndquadrant.com> wrote:
> >> The reason why I kept the single-word variant is consistency with other
> >> GUCs that affect planning, like enable_indexscan, enable_hashjoin and
> >> many others.
>
> > Right, so that makes sense, but from a larger point of view, how much
> > sense does it actually make?
>
> Maybe I'm just used to the names, but I find that things like
> "enable_seqscan" and "enable_nestloop" are pretty readable.
> Once they get longer, though, not so much. So I agree with
> renaming enable_incrementalsort.
I think the big problem is that, without the extra underscore, it reads
as increment-alsort. ;-)
--
Bruce Momjian <bruce@momjian.us> https://momjian.us
EnterpriseDB https://enterprisedb.com
The usefulness of a cup is in its emptiness, Bruce Lee
^ permalink raw reply [nested|flat] 14+ messages in thread
* Re: suggest to rename enable_incrementalsort
2020-06-21 06:26 suggest to rename enable_incrementalsort Peter Eisentraut <peter.eisentraut@2ndquadrant.com>
2020-06-21 07:05 ` Re: suggest to rename enable_incrementalsort Julien Rouhaud <rjuju123@gmail.com>
2020-06-21 11:21 ` Re: suggest to rename enable_incrementalsort Tomas Vondra <tomas.vondra@2ndquadrant.com>
2020-06-22 14:16 ` Re: suggest to rename enable_incrementalsort Robert Haas <robertmhaas@gmail.com>
2020-06-22 14:41 ` Re: suggest to rename enable_incrementalsort Tom Lane <tgl@sss.pgh.pa.us>
2020-06-22 15:22 ` Re: suggest to rename enable_incrementalsort Bruce Momjian <bruce@momjian.us>
@ 2020-06-22 16:16 ` Tom Lane <tgl@sss.pgh.pa.us>
1 sibling, 0 replies; 14+ messages in thread
From: Tom Lane @ 2020-06-22 16:16 UTC (permalink / raw)
To: Bruce Momjian <bruce@momjian.us>; +Cc: Robert Haas <robertmhaas@gmail.com>; Tomas Vondra <tomas.vondra@2ndquadrant.com>; Julien Rouhaud <rjuju123@gmail.com>; Peter Eisentraut <peter.eisentraut@2ndquadrant.com>; pgsql-hackers
Bruce Momjian <bruce@momjian.us> writes:
> On Mon, Jun 22, 2020 at 10:41:17AM -0400, Tom Lane wrote:
>> Maybe I'm just used to the names, but I find that things like
>> "enable_seqscan" and "enable_nestloop" are pretty readable.
>> Once they get longer, though, not so much. So I agree with
>> renaming enable_incrementalsort.
> I think the big problem is that, without the extra underscore, it reads
> as increment-alsort. ;-)
Yeah, the longer the name gets, the harder it is to see where the
word boundaries are.
regards, tom lane
^ permalink raw reply [nested|flat] 14+ messages in thread
* Re: suggest to rename enable_incrementalsort
2020-06-21 06:26 suggest to rename enable_incrementalsort Peter Eisentraut <peter.eisentraut@2ndquadrant.com>
2020-06-21 07:05 ` Re: suggest to rename enable_incrementalsort Julien Rouhaud <rjuju123@gmail.com>
2020-06-21 11:21 ` Re: suggest to rename enable_incrementalsort Tomas Vondra <tomas.vondra@2ndquadrant.com>
2020-06-22 14:16 ` Re: suggest to rename enable_incrementalsort Robert Haas <robertmhaas@gmail.com>
2020-06-22 14:41 ` Re: suggest to rename enable_incrementalsort Tom Lane <tgl@sss.pgh.pa.us>
2020-06-22 15:22 ` Re: suggest to rename enable_incrementalsort Bruce Momjian <bruce@momjian.us>
@ 2020-06-22 16:17 ` Robert Haas <robertmhaas@gmail.com>
2020-07-02 15:25 ` Re: suggest to rename enable_incrementalsort James Coleman <jtc331@gmail.com>
1 sibling, 1 reply; 14+ messages in thread
From: Robert Haas @ 2020-06-22 16:17 UTC (permalink / raw)
To: Bruce Momjian <bruce@momjian.us>; +Cc: Tom Lane <tgl@sss.pgh.pa.us>; Tomas Vondra <tomas.vondra@2ndquadrant.com>; Julien Rouhaud <rjuju123@gmail.com>; Peter Eisentraut <peter.eisentraut@2ndquadrant.com>; pgsql-hackers
On Mon, Jun 22, 2020 at 11:22 AM Bruce Momjian <bruce@momjian.us> wrote:
> I think the big problem is that, without the extra underscore, it reads
> as increment-alsort. ;-)
I know you're joking, but I think there's a serious issue here. We
often both omit word separators and also abbreviate, and I doubt that
the meaning is always obvious to people whose first language is
Japanese or Russian or something. The only human language other than
English in which I have any competence at all is Spanish, and if
somebody speaks Spanish to me the way that it's explained in a
textbook, I can understand it fairly well, especially if we're talking
about the kinds of topics that textbooks discuss rather than technical
stuff. But as soon as you start to use abbreviations or idioms, you're
going to lose me. Without a doubt, the best solution to this problem
would be for me to have better Spanish, but in the absence of that, on
those occasions when I need to communicate in Spanish, I sure do like
it when people are willing and able to make that as easy for me as
they can. I suspect other people have similar experiences.
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
^ permalink raw reply [nested|flat] 14+ messages in thread
* Re: suggest to rename enable_incrementalsort
2020-06-21 06:26 suggest to rename enable_incrementalsort Peter Eisentraut <peter.eisentraut@2ndquadrant.com>
2020-06-21 07:05 ` Re: suggest to rename enable_incrementalsort Julien Rouhaud <rjuju123@gmail.com>
2020-06-21 11:21 ` Re: suggest to rename enable_incrementalsort Tomas Vondra <tomas.vondra@2ndquadrant.com>
2020-06-22 14:16 ` Re: suggest to rename enable_incrementalsort Robert Haas <robertmhaas@gmail.com>
2020-06-22 14:41 ` Re: suggest to rename enable_incrementalsort Tom Lane <tgl@sss.pgh.pa.us>
2020-06-22 15:22 ` Re: suggest to rename enable_incrementalsort Bruce Momjian <bruce@momjian.us>
2020-06-22 16:17 ` Re: suggest to rename enable_incrementalsort Robert Haas <robertmhaas@gmail.com>
@ 2020-07-02 15:25 ` James Coleman <jtc331@gmail.com>
2020-07-05 10:20 ` Re: suggest to rename enable_incrementalsort Peter Eisentraut <peter.eisentraut@2ndquadrant.com>
0 siblings, 1 reply; 14+ messages in thread
From: James Coleman @ 2020-07-02 15:25 UTC (permalink / raw)
To: Robert Haas <robertmhaas@gmail.com>; +Cc: Bruce Momjian <bruce@momjian.us>; Tom Lane <tgl@sss.pgh.pa.us>; Tomas Vondra <tomas.vondra@2ndquadrant.com>; Julien Rouhaud <rjuju123@gmail.com>; Peter Eisentraut <peter.eisentraut@2ndquadrant.com>; pgsql-hackers
I think the change makes a lot of sense. The only reason I had it as
enable_incrementalsort in the first place was trying to broadly
following the existing GUC names, but as has already been pointed out,
there's a lot of variation there, and my version of the patch already
changed it to be more readable (at one point it was
enable_incsort...which is short...but does not have an obvious
meaning).
I've attached a patch to make the change, though if people are
interested in Tom's suggestion of enable_sort_incremental I could
switch to that.
James
Attachments:
v1-0001-Rename-enable_incrementalsort-for-clarity.patch (application/octet-stream, 11.0K) download
From 47f5e6221b2fa8e857d154ed0f0529c9254c5ca1 Mon Sep 17 00:00:00 2001
From: jcoleman <jtc331@gmail.com>
Date: Thu, 2 Jul 2020 15:20:58 +0000
Subject: [PATCH v1] Rename enable_incrementalsort for clarity
---
doc/src/sgml/config.sgml | 6 +++---
src/backend/optimizer/path/allpaths.c | 2 +-
src/backend/optimizer/path/costsize.c | 2 +-
src/backend/optimizer/plan/planner.c | 14 +++++++-------
src/backend/utils/misc/guc.c | 4 ++--
src/backend/utils/misc/postgresql.conf.sample | 2 +-
src/include/optimizer/cost.h | 2 +-
src/test/regress/expected/incremental_sort.out | 4 ++--
src/test/regress/expected/partition_aggregate.out | 2 +-
src/test/regress/expected/sysviews.out | 2 +-
src/test/regress/sql/incremental_sort.sql | 4 ++--
src/test/regress/sql/partition_aggregate.sql | 2 +-
12 files changed, 23 insertions(+), 23 deletions(-)
diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml
index b81aab239f..c4d5e8f3e4 100644
--- a/doc/src/sgml/config.sgml
+++ b/doc/src/sgml/config.sgml
@@ -4574,10 +4574,10 @@ ANY <replaceable class="parameter">num_sync</replaceable> ( <replaceable class="
</listitem>
</varlistentry>
- <varlistentry id="guc-enable-incrementalsort" xreflabel="enable_incrementalsort">
- <term><varname>enable_incrementalsort</varname> (<type>boolean</type>)
+ <varlistentry id="guc-enable-incrementalsort" xreflabel="enable_incremental_sort">
+ <term><varname>enable_incremental_sort</varname> (<type>boolean</type>)
<indexterm>
- <primary><varname>enable_incrementalsort</varname> configuration parameter</primary>
+ <primary><varname>enable_incremental_sort</varname> configuration parameter</primary>
</indexterm>
</term>
<listitem>
diff --git a/src/backend/optimizer/path/allpaths.c b/src/backend/optimizer/path/allpaths.c
index d984da25d7..81896de936 100644
--- a/src/backend/optimizer/path/allpaths.c
+++ b/src/backend/optimizer/path/allpaths.c
@@ -2912,7 +2912,7 @@ generate_useful_gather_paths(PlannerInfo *root, RelOptInfo *rel, bool override_r
* Consider incremental sort, but only when the subpath is already
* partially sorted on a pathkey prefix.
*/
- if (enable_incrementalsort && presorted_keys > 0)
+ if (enable_incremental_sort && presorted_keys > 0)
{
Path *tmp;
diff --git a/src/backend/optimizer/path/costsize.c b/src/backend/optimizer/path/costsize.c
index 4ff3c7a2fd..87c9b49ce1 100644
--- a/src/backend/optimizer/path/costsize.c
+++ b/src/backend/optimizer/path/costsize.c
@@ -128,7 +128,7 @@ bool enable_indexonlyscan = true;
bool enable_bitmapscan = true;
bool enable_tidscan = true;
bool enable_sort = true;
-bool enable_incrementalsort = true;
+bool enable_incremental_sort = true;
bool enable_hashagg = true;
bool hashagg_avoid_disk_plan = true;
bool enable_nestloop = true;
diff --git a/src/backend/optimizer/plan/planner.c b/src/backend/optimizer/plan/planner.c
index 4131019fc9..14f3fd44e3 100644
--- a/src/backend/optimizer/plan/planner.c
+++ b/src/backend/optimizer/plan/planner.c
@@ -5014,7 +5014,7 @@ create_ordered_paths(PlannerInfo *root,
* presorted the path is. Additionally incremental sort may enable
* a cheaper startup path to win out despite higher total cost.
*/
- if (!enable_incrementalsort)
+ if (!enable_incremental_sort)
continue;
/* Likewise, if the path can't be used for incremental sort. */
@@ -5095,7 +5095,7 @@ create_ordered_paths(PlannerInfo *root,
* sort_pathkeys because then we can't possibly have a presorted
* prefix of the list without having the list be fully sorted.
*/
- if (enable_incrementalsort && list_length(root->sort_pathkeys) > 1)
+ if (enable_incremental_sort && list_length(root->sort_pathkeys) > 1)
{
ListCell *lc;
@@ -6572,7 +6572,7 @@ add_paths_to_grouping_rel(PlannerInfo *root, RelOptInfo *input_rel,
* when the path is not already sorted and when incremental sort
* is enabled.
*/
- if (is_sorted || !enable_incrementalsort)
+ if (is_sorted || !enable_incremental_sort)
continue;
/* Restore the input path (we might have added Sort on top). */
@@ -6699,7 +6699,7 @@ add_paths_to_grouping_rel(PlannerInfo *root, RelOptInfo *input_rel,
* when the path is not already sorted and when incremental
* sort is enabled.
*/
- if (is_sorted || !enable_incrementalsort)
+ if (is_sorted || !enable_incremental_sort)
continue;
/* Restore the input path (we might have added Sort on top). */
@@ -7022,7 +7022,7 @@ create_partial_grouping_paths(PlannerInfo *root,
* group_pathkeys because then we can't possibly have a presorted
* prefix of the list without having the list be fully sorted.
*/
- if (enable_incrementalsort && list_length(root->group_pathkeys) > 1)
+ if (enable_incremental_sort && list_length(root->group_pathkeys) > 1)
{
foreach(lc, input_rel->pathlist)
{
@@ -7125,7 +7125,7 @@ create_partial_grouping_paths(PlannerInfo *root,
* when the path is not already sorted and when incremental sort
* is enabled.
*/
- if (is_sorted || !enable_incrementalsort)
+ if (is_sorted || !enable_incremental_sort)
continue;
/* Restore the input path (we might have added Sort on top). */
@@ -7304,7 +7304,7 @@ gather_grouping_paths(PlannerInfo *root, RelOptInfo *rel)
* group_pathkeys because then we can't possibly have a presorted prefix
* of the list without having the list be fully sorted.
*/
- if (!enable_incrementalsort || list_length(root->group_pathkeys) == 1)
+ if (!enable_incremental_sort || list_length(root->group_pathkeys) == 1)
return;
/* also consider incremental sort on partial paths, if enabled */
diff --git a/src/backend/utils/misc/guc.c b/src/backend/utils/misc/guc.c
index 75fc6f11d6..3a802d8627 100644
--- a/src/backend/utils/misc/guc.c
+++ b/src/backend/utils/misc/guc.c
@@ -983,11 +983,11 @@ static struct config_bool ConfigureNamesBool[] =
NULL, NULL, NULL
},
{
- {"enable_incrementalsort", PGC_USERSET, QUERY_TUNING_METHOD,
+ {"enable_incremental_sort", PGC_USERSET, QUERY_TUNING_METHOD,
gettext_noop("Enables the planner's use of incremental sort steps."),
NULL
},
- &enable_incrementalsort,
+ &enable_incremental_sort,
true,
NULL, NULL, NULL
},
diff --git a/src/backend/utils/misc/postgresql.conf.sample b/src/backend/utils/misc/postgresql.conf.sample
index 3a25287a39..0d98e546a6 100644
--- a/src/backend/utils/misc/postgresql.conf.sample
+++ b/src/backend/utils/misc/postgresql.conf.sample
@@ -361,7 +361,7 @@
#enable_parallel_append = on
#enable_seqscan = on
#enable_sort = on
-#enable_incrementalsort = on
+#enable_incremental_sort = on
#enable_tidscan = on
#enable_partitionwise_join = off
#enable_partitionwise_aggregate = off
diff --git a/src/include/optimizer/cost.h b/src/include/optimizer/cost.h
index 92e70ec0d9..613db8eab6 100644
--- a/src/include/optimizer/cost.h
+++ b/src/include/optimizer/cost.h
@@ -53,7 +53,7 @@ extern PGDLLIMPORT bool enable_indexonlyscan;
extern PGDLLIMPORT bool enable_bitmapscan;
extern PGDLLIMPORT bool enable_tidscan;
extern PGDLLIMPORT bool enable_sort;
-extern PGDLLIMPORT bool enable_incrementalsort;
+extern PGDLLIMPORT bool enable_incremental_sort;
extern PGDLLIMPORT bool enable_hashagg;
extern PGDLLIMPORT bool hashagg_avoid_disk_plan;
extern PGDLLIMPORT bool enable_nestloop;
diff --git a/src/test/regress/expected/incremental_sort.out b/src/test/regress/expected/incremental_sort.out
index 53accd0df9..e376ea1276 100644
--- a/src/test/regress/expected/incremental_sort.out
+++ b/src/test/regress/expected/incremental_sort.out
@@ -1414,7 +1414,7 @@ create table t (a int, b int, c int);
insert into t select mod(i,10),mod(i,10),i from generate_series(1,10000) s(i);
create index on t (a);
analyze t;
-set enable_incrementalsort = off;
+set enable_incremental_sort = off;
explain (costs off) select a,b,sum(c) from t group by 1,2 order by 1,2,3 limit 1;
QUERY PLAN
------------------------------------------------------
@@ -1430,7 +1430,7 @@ explain (costs off) select a,b,sum(c) from t group by 1,2 order by 1,2,3 limit 1
-> Parallel Seq Scan on t
(10 rows)
-set enable_incrementalsort = on;
+set enable_incremental_sort = on;
explain (costs off) select a,b,sum(c) from t group by 1,2 order by 1,2,3 limit 1;
QUERY PLAN
----------------------------------------------------------------------
diff --git a/src/test/regress/expected/partition_aggregate.out b/src/test/regress/expected/partition_aggregate.out
index c36970575f..45c698daf4 100644
--- a/src/test/regress/expected/partition_aggregate.out
+++ b/src/test/regress/expected/partition_aggregate.out
@@ -12,7 +12,7 @@ SET enable_partitionwise_join TO true;
-- Disable parallel plans.
SET max_parallel_workers_per_gather TO 0;
-- Disable incremental sort, which can influence selected plans due to fuzz factor.
-SET enable_incrementalsort TO off;
+SET enable_incremental_sort TO off;
--
-- Tests for list partitioned tables.
--
diff --git a/src/test/regress/expected/sysviews.out b/src/test/regress/expected/sysviews.out
index 01b7786f01..06c4c3e476 100644
--- a/src/test/regress/expected/sysviews.out
+++ b/src/test/regress/expected/sysviews.out
@@ -76,7 +76,7 @@ select name, setting from pg_settings where name like 'enable%';
enable_gathermerge | on
enable_hashagg | on
enable_hashjoin | on
- enable_incrementalsort | on
+ enable_incremental_sort | on
enable_indexonlyscan | on
enable_indexscan | on
enable_material | on
diff --git a/src/test/regress/sql/incremental_sort.sql b/src/test/regress/sql/incremental_sort.sql
index 373e62ac13..9c040c90e6 100644
--- a/src/test/regress/sql/incremental_sort.sql
+++ b/src/test/regress/sql/incremental_sort.sql
@@ -210,10 +210,10 @@ insert into t select mod(i,10),mod(i,10),i from generate_series(1,10000) s(i);
create index on t (a);
analyze t;
-set enable_incrementalsort = off;
+set enable_incremental_sort = off;
explain (costs off) select a,b,sum(c) from t group by 1,2 order by 1,2,3 limit 1;
-set enable_incrementalsort = on;
+set enable_incremental_sort = on;
explain (costs off) select a,b,sum(c) from t group by 1,2 order by 1,2,3 limit 1;
-- Incremental sort vs. set operations with varno 0
diff --git a/src/test/regress/sql/partition_aggregate.sql b/src/test/regress/sql/partition_aggregate.sql
index 178f2079fa..117f65ecb4 100644
--- a/src/test/regress/sql/partition_aggregate.sql
+++ b/src/test/regress/sql/partition_aggregate.sql
@@ -13,7 +13,7 @@ SET enable_partitionwise_join TO true;
-- Disable parallel plans.
SET max_parallel_workers_per_gather TO 0;
-- Disable incremental sort, which can influence selected plans due to fuzz factor.
-SET enable_incrementalsort TO off;
+SET enable_incremental_sort TO off;
--
-- Tests for list partitioned tables.
--
2.20.1
* Re: suggest to rename enable_incrementalsort
2020-06-21 06:26 suggest to rename enable_incrementalsort Peter Eisentraut <peter.eisentraut@2ndquadrant.com>
2020-06-21 07:05 ` Re: suggest to rename enable_incrementalsort Julien Rouhaud <rjuju123@gmail.com>
2020-06-21 11:21 ` Re: suggest to rename enable_incrementalsort Tomas Vondra <tomas.vondra@2ndquadrant.com>
2020-06-22 14:16 ` Re: suggest to rename enable_incrementalsort Robert Haas <robertmhaas@gmail.com>
2020-06-22 14:41 ` Re: suggest to rename enable_incrementalsort Tom Lane <tgl@sss.pgh.pa.us>
2020-06-22 15:22 ` Re: suggest to rename enable_incrementalsort Bruce Momjian <bruce@momjian.us>
2020-06-22 16:17 ` Re: suggest to rename enable_incrementalsort Robert Haas <robertmhaas@gmail.com>
2020-07-02 15:25 ` Re: suggest to rename enable_incrementalsort James Coleman <jtc331@gmail.com>
@ 2020-07-05 10:20 ` Peter Eisentraut <peter.eisentraut@2ndquadrant.com>
0 siblings, 0 replies; 14+ messages in thread
From: Peter Eisentraut @ 2020-07-05 10:20 UTC (permalink / raw)
To: James Coleman <jtc331@gmail.com>; Robert Haas <robertmhaas@gmail.com>; +Cc: Bruce Momjian <bruce@momjian.us>; Tom Lane <tgl@sss.pgh.pa.us>; Tomas Vondra <tomas.vondra@2ndquadrant.com>; Julien Rouhaud <rjuju123@gmail.com>; pgsql-hackers
On 2020-07-02 17:25, James Coleman wrote:
> I think the change makes a lot of sense. The only reason I had it as
> enable_incrementalsort in the first place was trying to broadly
> following the existing GUC names, but as has already been pointed out,
> there's a lot of variation there, and my version of the patch already
> changed it to be more readable (at one point it was
> enable_incsort...which is short...but does not have an obvious
> meaning).
>
> I've attached a patch to make the change,
committed
--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
^ permalink raw reply [nested|flat] 14+ messages in thread
end of thread, other threads:[~2020-07-05 10:20 UTC | newest]
Thread overview: 14+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2020-06-21 06:26 suggest to rename enable_incrementalsort Peter Eisentraut <peter.eisentraut@2ndquadrant.com>
2020-06-21 07:05 ` Julien Rouhaud <rjuju123@gmail.com>
2020-06-21 11:21 ` Tomas Vondra <tomas.vondra@2ndquadrant.com>
2020-06-21 23:18 ` David Rowley <dgrowleyml@gmail.com>
2020-06-22 11:55 ` Ashutosh Bapat <ashutosh.bapat.oss@gmail.com>
2020-06-22 14:16 ` Robert Haas <robertmhaas@gmail.com>
2020-06-22 14:31 ` Tomas Vondra <tomas.vondra@2ndquadrant.com>
2020-06-22 15:13 ` Robert Haas <robertmhaas@gmail.com>
2020-06-22 14:41 ` Tom Lane <tgl@sss.pgh.pa.us>
2020-06-22 15:22 ` Bruce Momjian <bruce@momjian.us>
2020-06-22 16:16 ` Tom Lane <tgl@sss.pgh.pa.us>
2020-06-22 16:17 ` Robert Haas <robertmhaas@gmail.com>
2020-07-02 15:25 ` James Coleman <jtc331@gmail.com>
2020-07-05 10:20 ` Peter Eisentraut <peter.eisentraut@2ndquadrant.com>
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox