Обсуждение: Re: [COMMITTERS] pgsql: Can't completely get rid of #ifndef FRONTEND in palloc.h :-(

Tom Lane wrote:
> Can't completely get rid of #ifndef FRONTEND in palloc.h :-(
>
> pg_controldata includes postgres.h not postgres_fe.h, so utils/palloc.h
> must be able to compile in a "#define FRONTEND" context.

Hmm, I had this patch in an abandoned branch from long ago, which I
think helped remove postgres.h from pg_controldata.  I remembered it
just now because of this commit message.  Maybe it's useful to re-remove
the #ifndef FRONTEND from palloc.h.

It's not rebased to latest master and maybe even not complete; if people
think this approach is worthwhile I can try and clean it up and
proposely more seriously; LMK.  (Also if people think it needs futher
tweaks.  I vaguely recall I didn't propose it back then because the set
of stuff in the new header could be tweaked.)

--
Álvaro Herrera                http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services

Alvaro Herrera <alvherre@2ndquadrant.com> writes:
> Tom Lane wrote:
>> pg_controldata includes postgres.h not postgres_fe.h, so utils/palloc.h
>> must be able to compile in a "#define FRONTEND" context.

> Hmm, I had this patch in an abandoned branch from long ago, which I
> think helped remove postgres.h from pg_controldata.  I remembered it
> just now because of this commit message.  Maybe it's useful to re-remove
> the #ifndef FRONTEND from palloc.h.

Hm.  It would certainly be better if pg_controldata could use
postgres_fe.h not postgres.h, but I'm confused about how the new header
added by this patch helps that?  None of the declarations you removed from
xlog.h look like they'd be more problematic than the ones you left behind.
        regards, tom lane

Tom Lane wrote:
> Alvaro Herrera <alvherre@2ndquadrant.com> writes:
> > Tom Lane wrote:
> >> pg_controldata includes postgres.h not postgres_fe.h, so utils/palloc.h
> >> must be able to compile in a "#define FRONTEND" context.
> 
> > Hmm, I had this patch in an abandoned branch from long ago, which I
> > think helped remove postgres.h from pg_controldata.  I remembered it
> > just now because of this commit message.  Maybe it's useful to re-remove
> > the #ifndef FRONTEND from palloc.h.
> 
> Hm.  It would certainly be better if pg_controldata could use
> postgres_fe.h not postgres.h, but I'm confused about how the new header
> added by this patch helps that?  None of the declarations you removed from
> xlog.h look like they'd be more problematic than the ones you left behind.

The point IIRC was to be able to remove stuff that required type Datum
to be defined:

-extern void ShutdownXLOG(int code, Datum arg);

which led to the idea that the new header would be about process control
for xlog.c.

-- 
Álvaro Herrera                http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services

Alvaro Herrera <alvherre@2ndquadrant.com> writes:
> Tom Lane wrote:
>> Hm.  It would certainly be better if pg_controldata could use
>> postgres_fe.h not postgres.h, but I'm confused about how the new header
>> added by this patch helps that?  None of the declarations you removed from
>> xlog.h look like they'd be more problematic than the ones you left behind.

> The point IIRC was to be able to remove stuff that required type Datum
> to be defined:

> -extern void ShutdownXLOG(int code, Datum arg);

> which led to the idea that the new header would be about process control
> for xlog.c.

Oh, Datum was the issue?  I see.  But then this approach is basically
requiring that we can never reference Datum in any header that
pg_controldata uses.  Which seems like a pretty draconian limitation,
particularly if the headers involved are general-purpose ones like xlog.h.

What might be less breakable is to identify exactly which declarations
pg_controldata needs out of this file and push just those into some new
header.

However, pg_controldata is just the tip of the iceberg --- to get any
mileage out of this, we'd also have to make pg_resetxlog and pg_xlogdump
able to compile with only postgres_fe.h, so there might be too much stuff
involved.
        regards, tom lane

Tom Lane wrote:

> What might be less breakable is to identify exactly which declarations
> pg_controldata needs out of this file and push just those into some new
> header.

Yeah, I also thought about that but didn't get around to trying.

> However, pg_controldata is just the tip of the iceberg --- to get any
> mileage out of this, we'd also have to make pg_resetxlog and pg_xlogdump
> able to compile with only postgres_fe.h, so there might be too much stuff
> involved.

IIRC we didn't have pg_xlogdump when I wrote this, and I checked
pg_resetxlog and it wasn't nearly as simple as pg_controldata.

-- 
Álvaro Herrera                http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services