On Mon, Jun 22, 2020 at 4:48 AM David Rowley <dgrowle...@gmail.com> wrote: > > On Sun, 21 Jun 2020 at 23:22, Tomas Vondra <tomas.von...@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.eisentr...@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
