Re: postgres.h included from relcache.h - but removing it breaks pg_upgrade

Tom Lane <tgl@sss.pgh.pa.us>

From: Tom Lane <tgl@sss.pgh.pa.us>
To: Andres Freund <andres@anarazel.de>
Cc: pgsql-hackers@postgresql.org, Alexander Korotkov <akorotkov@postgresql.org>
Date: 2021-09-14T03:53:24Z
Lists: pgsql-hackers
Andres Freund <andres@anarazel.de> writes:
> On 2021-09-13 22:40:19 -0400, Tom Lane wrote:
>> As for the fix ... what in the world is pg_upgrade doing including
>> relcache.h?  It seems like there's a more fundamental problem here:
>> either relcache.h is declaring something that needs to be elsewhere,
>> or pg_upgrade is doing something it should not.

> We could split visibilitymap.h into two, or we could forward-declare Relation
> and not include relcache...

Without having looked at the details, I think using a forward-declare
to avoid including relcache.h in visibilitymap.h might be a reasonably
non-painful fix.  OTOH, in the long run it might be worth the effort
to split visibilitymap.h to separate useful file-contents knowledge
from backend function declarations.

>> No.  If anything, I'd want to throw an error for "redundant" includes
>> of these files, because it's a pretty good red flag about
>> poorly-thought-out header modularization.

> I think we might be thinking of the same. What I meant with "avoid" was to
> raise a warning or error.

Ah, we are on the same page then.  I misunderstood what you wrote.

> If we were to do that, it's probably worth doing the
> build system ugliness to do this only when building postgres code, rather than
> extensions...

As long as we do this in HEAD only, I'm not sure why extensions
need an exception.  Perhaps it will result in somebody pointing out
additional poorly-thought-out header contents, but I don't think
that's bad.

			regards, tom lane



Commits

  1. Split macros from visibilitymap.h into a separate header

  2. Implement operator class parameters