On Mon, Sep 16, 2013 at 11:02:28PM -0300, Alvaro Herrera wrote:
> relscan.h is a bit of an unfortunate header because it requires a lot of
> other headers to compile, and is also required by execnodes.h. This
Not in my copy of the source tree. execnodes.h uses the corresponding
typedefs that appear in genam.h and heapam.h. Indeed, "find . -name '*.Po' |
xargs grep -l relscan.h | wc -l" only locates 26 files that include relscan.h
directly or indirectly.
> quick patch removes the struct definitions from that file, moving them
> into a new relscan_details.h file; the reworked relscan.h does not need
> any other header, freeing execnodes.h from needlessly including lots of
> unnecessary stuff all over the place. Only the files that really need
> the struct definition include the new file, and as seen in the patch,
> they are quite few.
>
> execnodes.h is included by 147 backend source files; relscan_details.h
> only 27. So the exposure area is reduced considerably. This patch also
> modifies a few other backend source files to add one or both of genam.h
> and heapam.h, which were previously included through execnodes.h.
>
> This is probably missing tweaks for contrib/. The following modules
> would need to include relscan_details.h: sepgsql dblink pgstattuple
> pgrowlocks. I expect the effect on third-party modules to be much less
> than by the htup_details.h change.
-1 on header restructuring in the absence of noteworthy compile-time benchmark
improvements. Besides the obvious benchmark of full-build time, one could
exercise the benefit of fewer header dependencies by modelling the series of
compile times from running "git pull; make" at each commit for the past
several months. The latter benchmark is perhaps more favorable.
nm
--
Noah Misch
EnterpriseDB http://www.enterprisedb.com