Re: suggest to rename enable_incrementalsort

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