Thread: 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
Attachment
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@vmware.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