Re: Proposed refactoring of planner header files

Tom Lane <tgl@sss.pgh.pa.us>

From: Tom Lane <tgl@sss.pgh.pa.us>
To: Robert Haas <robertmhaas@gmail.com>
Cc: PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
Date: 2019-01-31T20:28:57Z
Lists: pgsql-hackers
Robert Haas <robertmhaas@gmail.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


Commits

  1. Refactor index cost estimation functions in view of IndexClause changes.

  2. Simplify the planner's new representation of indexable clauses a little.

  3. Move pattern selectivity code from selfuncs.c to like_support.c.

  4. Refactor planner's header files.

  5. Make some small planner API cleanups.