Re: Sorting case branches in outfuncs.c/outNode alphabetically

Fedir Panasenko <fpanasenko(at)gmail(dot)com> writes:
> outfuncs.c contains a switch statement responsible for choosing
> serialization function per node type here:
> https://github.com/postgres/postgres/blob/master/src/backend/nodes/outfuncs.c#L3711

Why are you concerned about outfuncs.c in particular? Its sibling files
(copyfuncs, equalfuncs, etc) have much the same structure.

> It spans over >650LOC and is quite unreadable, requiring using search
> or code analysis tools for pretty much anything.

But why exactly do you need to read it? It's boring boilerplate.

> I'd like to sort these case branches alphabetically and I'd like to
> get some input on that prior to submitting a patch.

I'd be a strong -1 for alphabetical sort. To my mind, the entries
here, and in other similar places, should match the order in which the
struct types are declared in src/include/nodes/*nodes.h. And those
are not sorted alphabetically, but (more or less) by functionality.
I would *definitely* not favor a patch that arbitrarily re-orders
those header files alphabetically.

Now, IIRC the ordering in the backend/nodes/*.c files is not always
a perfect match to the headers. I'd be good with a patch that makes
them line up better. But TBH, that is just neatnik-ism; I still don't
see how it makes any interesting difference to readability.

Keep in mind also that various people have shown interest in
auto-generating the backend/nodes/*.c files from the header
declarations, in which case this discussion would be moot.

> (readfuncs.c contains a similar construct for deserializing nodes, but
> that one is if...else based as opposed to switch, so order there might
> have performance implications -> I'd reserve that topic for separate
> discussion).

Yeah, I keep wondering when that structure is going to become a
noticeable performance problem. There's little reason to think that
we've ordered the node types by frequency there. At some point it might
make sense to convert readfuncs' lookup logic into, say, a perfect hash
table (cf src/tools/PerfectHash.pm). I'd certainly think that that
would be a more useful activity than arguing over the switch order.

regards, tom lane