I noticed (by running "cd src/include ; make check" with the attached
patch applied) that since commit b89e151054 ("Introduce logical
decoding.") tqual.h now emits this warning when compiled standalone:
/pgsql/source/HEAD/src/include/utils/tqual.h:101:13: warning: ‘struct HTAB’ declared inside parameter list [enabled by
default]
/pgsql/source/HEAD/src/include/utils/tqual.h:101:13: warning: its scope is only this definition or declaration, which
isprobably not what you want [enabled by default]
The prototype in question is:
/** To avoid leaking to much knowledge about reorderbuffer implementation* details this is implemented in
reorderbuffer.cnot tqual.c.*/
extern bool ResolveCminCmaxDuringDecoding(HTAB *tuplecid_data, Snapshot
snapshot, HeapTuple htup, Buffer buffer,
CommandId *cmin, CommandId *cmax);
Now there are two easy ways to silence this:
1. add "struct HTAB;" to the previous line in tqual.h, i.e. a forward
declaration. We have very few of these; most of the time ISTM we prefer
a solution like the next one:
2. add #include "utils/hsearch.h" at the top of tqual.h.
Now my inclination is to use the second one, because it seems cleaner to
me and avoid having a forward struct decl in an unrelated header, which
will later bleed into other source files. Do we have a project
guideline saying that we prefer option two, and does that explain why we
have so few declarations as in option one?
There is of course a third choice which is to dictate that this function
ought to be declared in reorderbuffer.h; but that would have the
unpleasant side-effect that tqual.c would need to #include that.
Opinions please? I would like to have a guideline about this so I can
reason in a principled way in future cases in which this kind of
question arises.
--
Álvaro Herrera http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services