Re: suggest to rename enable_incrementalsort

On Sun, 21 Jun 2020 at 23:22, Tomas Vondra <tomas(dot)vondra(at)2ndquadrant(dot)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(dot)eisentraut(at)2ndquadrant(dot)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