Re: Proposed refactoring of planner header files

Robert Haas <robertmhaas(at)gmail(dot)com> writes:
> I do not have a powerful opinion on exactly what to do here, but I
> offer the following for what it's worth:

> - I do not really think much of the selfuncs.c naming. Nobody who is
> not deeply immersed in the PostgreSQL code knows what a "selfunc" is.
> Therefore, breaking selfuncs.c into opr_selfuncs.c and
> index_selfuncs.c doesn't strike me as the ideal naming choice. I
> would suggest looking for a name that is less steeped in venerable
> tradition; perhaps estimator.c or indexsupport.c or selectivity.c or
> whatever could be considered for whatever new files we are creating.

Hm. I'm not really persuaded. There isn't any part of this code that
isn't "steeped in venerable tradition"; the problem I'm ultimately
trying to fix here can fairly be characterized as 25-year-old technical
debt. I see your point that adt/ is not where new people might look for
selectivity estimators, but we should also have some sympathy for people
who *do* know the code base expecting to find these functions more or
less where they are.

> - I have always found the placement of selfuncs.c a bit strange: why
> is it in utils/adt? Perhaps there is an argument for having the
> things that are specific to a particular type or a particular operator
> in that directory, but if so, shouldn't they be grouped with the type
> or operator to which they relate, rather than each other?

We do already have that to some extent:

src/backend/tsearch/ts_selfuncs.c
src/backend/utils/adt/array_selfuncs.c
src/backend/utils/adt/rangetypes_selfuncs.c
src/backend/utils/adt/geo_selfuncs.c
src/backend/utils/adt/network_selfuncs.c
src/backend/utils/adt/selfuncs.c
src/include/utils/index_selfuncs.h
src/include/utils/selfuncs.h
contrib/intarray/_int_selfuncs.c

Breaking up selfuncs.c would carry this a bit further, but giving it
a random new name doesn't seem to me to really improve matters.

Now, if we move the support functions to someplace under optimizer/,
we do have to think about a name for that. I was considering
optimizer/utils/selsupport.c and optimizer/selsupport.h, but I'm
certainly not wedded to that.

regards, tom lane