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
Date: Thu, 31 Jan 2019 14:45:58 -0500
Message-ID: <11671.1548963958@sss.pgh.pa.us> (raw)
In-Reply-To: <CA+Tgmoa9kQZJ1eCDaiupQ5ZVW3c65YxnoE1NwwwdZzO=0gp=mw@mail.gmail.com>
References: <11460.1548706639@sss.pgh.pa.us>
<30320.1548904075@sss.pgh.pa.us>
<CA+Tgmoa9kQZJ1eCDaiupQ5ZVW3c65YxnoE1NwwwdZzO=0gp=mw@mail.gmail.com>
Robert Haas <robertmhaas@gmail.com> writes:
> On Wed, Jan 30, 2019 at 10:08 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> However ... there are three pretty clearly identifiable groups of
>> functions in selfuncs.c: operator-specific estimators, support
>> functions, and AM-specific indexscan cost estimation functions.
>> There's a case to be made for splitting them into three separate files.
> I think splitting it along these lines would be pretty sensible. I'm
> not really a fan of giant source files, especially if there's no
> really good reason to put all that stuff in one source file ... then
> again, I'm also not volunteering to do the work.
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 think 2b would make more sense in the abstract, but it would also
result in more #include churn for extensions. OTOH we already
forced some churn in that area with the planner-header-refactoring
patch, so the delta cost is probably not that much.
In any case, I'm hoping to get rid of the exported declarations for
pattern selectivity (Pattern_Prefix_Status, pattern_fixed_prefix,
etc) altogether. Those are exported because indxpath.c currently
calls them from its wired-in lossy-index-clause logic. When the
dust settles, all that code should be associated with planner
support functions for the LIKE and regex operators, and will not
need to be visible outside of whatever module we put those in.
But that patch is yet to be written.
Opinions?
regards, tom lane
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-hackers@postgresql.org
Cc: tgl@sss.pgh.pa.us, robertmhaas@gmail.com, pgsql-hackers@lists.postgresql.org
Subject: Re: Proposed refactoring of planner header files
In-Reply-To: <11671.1548963958@sss.pgh.pa.us>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox