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

Hi,

On 2021-09-13 22:40:19 -0400, Tom Lane wrote:
> Andres Freund <andres(at)anarazel(dot)de> writes:
> > I noticed that postgres.h is included from relcache.h (starting in [1]) and
> > wanted to fix that - it violates our usual policy against including postgres.h
> > from within headers.
>
> Ugh, yeah, that's entirely against policy.
>
> 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.

It's not directly including relcache. pg_upgrade/file.c needs a few symbols
from visibilitymap.h, for the upgrade of the visibilitymap from old to new
format. And visibilitymap needs relcache.h because several of its routines
take Relation params.

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

> > I was also wondering if we should put something in c.h and postgres.h to avoid
> > redundant includes?
>
> 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. 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...

Greetings,

Andres Freund