Re: warning when compiling utils/tqual.h

On Mon, Mar 17, 2014 at 12:55 PM, Andres Freund <andres@2ndquadrant.com> wrote:
> On 2014-03-17 12:50:37 -0400, Tom Lane wrote:
>> Alvaro Herrera <alvherre@2ndquadrant.com> writes:
>> > 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
[enabledby default]
 
>> > /pgsql/source/HEAD/src/include/utils/tqual.h:101:13: warning: its scope is only this definition or declaration,
whichis probably 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.c not tqual.c.
>> >  */
>> > extern bool ResolveCminCmaxDuringDecoding(HTAB *tuplecid_data,
>> >                                           Snapshot snapshot,
>> >                                           HeapTuple htup,
>> >                                           Buffer buffer,
>> >                                           CommandId *cmin, CommandId *cmax);
>>
>> I guess the real question is why such a prototype is in tqual.h in
>> the first place.  ISTM this should be pushed somewhere specific to
>> reorderbuffer.c.  I'm -1 on having struct HTAB bleed into tqual.h
>> via either of the methods you suggest.
>
> It's the only logical decoding specific routine that's called by tqual.c
> which doesn't need anything else from reorderbuffer. I'd like to avoid
> more stuff bleeding in there...

That's 100% irrelevant.  Function declarations for backend/X/Y/Z.c are
supposed to be in include/X/Z.h with a few historical exceptions I
hope we won't add to.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company