On Thu, Jan 31, 2019 at 2:46 PM Tom Lane <t...@sss.pgh.pa.us> wrote: > I'm happy to do the work if there's consensus on what to do. After > a few moments' thought, I'd suggest: > > 1. Move the indexscan cost estimation functions into a new file > adt/index_selfuncs.c. Most of them already are declared in > utils/index_selfuncs.h, and we'd move the remaining declarations > (primarily, genericcostestimate()) to that file as well. This > would affect #includes in contrib/bloom/ and any 3rd-party index > AMs that might be using genericcostestimate(). > > 2a. Leave the support functions in selfuncs.c with declarations in > utils/selfuncs.h, and move the operator-specific estimators to > a new file, perhaps opr_selfuncs.c. This would minimize pain for > existing includers of selfuncs.h, most of which (particularly outside > core) are presumably interested in the support functions. > > 2b. Alternatively, leave the operator-specific estimators where they > are, and make new files under optimizer/ for the support functions.
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. - 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? And if they're not specific to a certain thing, or if grouping them with that thing would suck because of what we'd have to #include or some other grounds, then why not put them in the optimizer? I think that a large fraction of this code is actually generic, and putting it in the optimizer directory someplace would be more logical than what we have now. Alternatively, since these have to be exposed via pg_proc, maybe the shims should go in this directory and the core logic elsewhere. Again, I don't have a strong opinion here, but I've never thought this made a ton of sense the way it is. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
