Re: Shuffling xlog header files

Andres Freund's xlogreader patch contains a change to move the
declarations of SQL-callable functions in xlogfuncs.c to a new header
file, xlog_fn.h. The purpose is to allow xlog_internal.h to be included
in a frontend program, as the PG_FUNCTION_ARGS and Datum used in the
prototypes require fmgr.h, which is backend-only. I think his patch
missed a trick: pg_basebackup, pg_controlinfo and pg_resetxlog currently
use this for the same purpose:

/*
* We have to use postgres.h not postgres_fe.h here, because there's so
much
* backend-only stuff in the XLOG include files we need. But we need a
* frontend-ish environment otherwise. Hence this ugly hack.
*/
#define FRONTEND 1

#include "postgres.h"
...
#include "access/xlog.h"

But this got me thinking whether we should do the xlog_fn.h refactoring,
so that we could get rid of that ugly hack in the existing programs,
too. I ended up with the attached. It allows xlog_internal.h to be
included in frontend programs. The name xlog_internal.h is a bit of a
misnomer now, it's not very internal anymore, if it can now actually be
included by external programs. But the point is that the file contains
declarations related to the WAL file format.

We still need the "#define FRONTEND 1" ugly hack in pg_controldata and
pg_resetxlog with this, but we get rid of it in pg_basebackup. I think
that's reasonable, pg_controldata and pg_resetxlog are more low-level
programs than pg_basebackup.

Any objections?

- Heikki

Hi,

The xlog_fn.h patch was Alvaro's idea (and patch) btw, I previously had
used an ugly typedef for Datum to get arround defining
FRONTEND/including postgres.h...

On 2012-12-10 19:56:49 +0200, Heikki Linnakangas wrote:
> We still need the "#define FRONTEND 1" ugly hack in pg_controldata and
> pg_resetxlog with this, but we get rid of it in pg_basebackup. I think
> that's reasonable, pg_controldata and pg_resetxlog are more low-level
> programs than pg_basebackup.

It's also for the pg_receivellog introduced in one of my other
patches... So it seems to be a good idea to me.

> The name xlog_internal.h is a bit of a misnomer now, it's
> not very internal anymore, if it can now actually be included by external
> programs. But the point is that the file contains declarations related to
> the WAL file format.

We could rename it to xlog_details.h or such, but I guess the noise
would outhweigh the benefits.

> Any objections?

Unsurprisingly none from my side.

Greetings,

Andres Freund

--
Andres Freund http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services

On 10 December 2012 17:56, Heikki Linnakangas <hlinnakangas(at)vmware(dot)com> wrote:

> Any objections?

No objections, though I'm concerned to make as few changes as possible. Thanks

--
Simon Riggs http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services

Heikki Linnakangas wrote:
> Andres Freund's xlogreader patch contains a change to move the
> declarations of SQL-callable functions in xlogfuncs.c to a new
> header file, xlog_fn.h. The purpose is to allow xlog_internal.h to
> be included in a frontend program, as the PG_FUNCTION_ARGS and Datum
> used in the prototypes require fmgr.h, which is backend-only. I
> think his patch missed a trick: pg_basebackup, pg_controlinfo and
> pg_resetxlog currently use this for the same purpose:

Yeah, I noticed that after I proposed that. I furthermore realized that
the whole xlog.h / xlog_internal.h split wasn't thorough enough; I think
there's some more useful reshuffling to be done in the xlog headers; in
particular I considered the idea that backend .c files that only need to
build and insert xlog records could use a very lean header that defined
only enough to get that task done, while others such as checkpointer,
walreceiver and sender (probably others) would require the more
extensive header.

I was a bit pressed for time so didn't get around to actually trying it
out.

So I don't object to your patch, but I do think that maybe we can go
even a bit further than this.

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