From: Tom Lane <tgl@sss.pgh.pa.us>
To: Robert Haas <robertmhaas@gmail.com>
cc: PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
Subject: Re: Proposed refactoring of planner header files
In-reply-to: 
 <CA+TgmoaZNN5Xb9SfiQEeFdL+zPdqmEMym8TX7Cecfae46ZSf+Q@mail.gmail.com>
References: <11460.1548706639@sss.pgh.pa.us> <30320.1548904075@sss.pgh.pa.us>
 <CA+Tgmoa9kQZJ1eCDaiupQ5ZVW3c65YxnoE1NwwwdZzO=0gp=mw@mail.gmail.com>
 <11671.1548963958@sss.pgh.pa.us>
 <CA+TgmoaZNN5Xb9SfiQEeFdL+zPdqmEMym8TX7Cecfae46ZSf+Q@mail.gmail.com>
Comments: In-reply-to Robert Haas <robertmhaas@gmail.com>
	message dated "Thu, 31 Jan 2019 15:10:17 -0500"
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-ID: <9002.1548966537.1@sss.pgh.pa.us>
Date: Thu, 31 Jan 2019 15:28:57 -0500
Message-ID: <9003.1548966537@sss.pgh.pa.us>
Precedence: bulk

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