Thread

1. Shuffling xlog header files

   Heikki Linnakangas <hlinnakangas@vmware.com> — 2012-12-10T17:56:49Z

   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
   

2. Re: Shuffling xlog header files

   Andres Freund <andres@2ndquadrant.com> — 2012-12-10T18:29:16Z

   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
   
   
   

3. Re: Shuffling xlog header files

   Simon Riggs <simon@2ndquadrant.com> — 2012-12-10T20:41:13Z

   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
   
   
   

4. Re: Shuffling xlog header files

   Alvaro Herrera <alvherre@2ndquadrant.com> — 2012-12-12T04:41:12Z

   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