[HACKERS] pg_waldump's inclusion of backend headers is a mess - Mailing list pgsql-hackers

From Robert Haas
Subject [HACKERS] pg_waldump's inclusion of backend headers is a mess
Date
Msg-id CA+TgmoZ=F=GkxV0YEv-A8tb+AEGy_Qa7GSiJ8deBKFATnzfEug@mail.gmail.com
Whole thread Raw
Responses Re: [HACKERS] pg_waldump's inclusion of backend headers is a mess  (Michael Paquier <michael.paquier@gmail.com>)
Re: [HACKERS] pg_waldump's inclusion of backend headers is a mess  (Andres Freund <andres@anarazel.de>)
List pgsql-hackers
Dilip complained on another thread about the apparent difficulty of
getting the system to compile after adding a reference to dsa_pointer
to tidbitmap.c:

https://www.postgresql.org/message-id/CAFiTN-u%3DBH_b0QE9FG%2B%2B_Ht6Q%3DF84am%3DJ-rt7fNbQkq3%2BqX3VQ%40mail.gmail.com

I dug into the problem and discovered that pg_waldump is slurping up a
tremendous crapload of backend headers.  It includes gin.h,
gist_private.h, hash_xlog.h, nbtree.h, and spgist.h, which all end up
including amapi.h, which includes genam.h, which includes tidbitmap.h.
When you make tidbitmap.h include utils/dsa.h, it in turn includes
port/atomics.h, which has an #error preventing frontend includes.

There are a number of ways to fix this problem; probably the cheapest
available hack is to stick #ifndef FRONTEND around the additional
stuff getting added to tidbitmap.h.  But that seems to be attacking
the problem from the wrong end.  The problem isn't really that having
tidbitmap.h refer to backend-only stuff is unreasonable; it's that
pg_waldump is skating on thin ice by importing so many backend-only
headers.  It's only a matter of time before some other one of the many
files that it directly or indirectly includes develops a similar
problem.

Therefore, I proposed the attached patch, which splits spgxlog.h out
of spgist_private.h, nbtxlog.h out of nbtree.h, gistxlog.h out of
gist_private.h, and ginxlog.h and ginblock.h out of gin_private.h.
These new header files are included by pg_waldump in lieu of the
"private" versions.  This solves the immediate problem and I suspect
it will head off future problems as well.

Thoughts, comments, objections, better ideas?

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

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Attachment

pgsql-hackers by date:

Previous
From: Michael Paquier
Date:
Subject: Re: [HACKERS] Provide list of subscriptions and publications inpsql's completion
Next
From: Robert Haas
Date:
Subject: Re: [HACKERS] log_autovacuum_min_duration doesn't log VACUUMs