On Mon, Sep 15, 2014 at 7:03 PM, Alvaro Herrera
<alvherre@2ndquadrant.com> wrote:
> Heikki Linnakangas wrote:
>
>> Here we go. I've split this again into two patches. The first patch
>> is just refactoring the current code. It moves XLogInsert into a new
>> file, xloginsert.c, and the definition of XLogRecord to new
>> xlogrecord.h header file. As a result, there is a a lot of churn in
>> the #includes in C files that generate WAL records, or contain redo
>> routines. The number of files that pull in xlog.h - directly or
>> indirectly through other headers - is greatly reduced.
>
> I think you should push the first patch for now and continue to
> investigate the issues in the second patch. Some minor suggestions I
> have for the first patch are that since xlog.h is no longer used by
> other headers (but only by .c files), you should just #include
> xloginsert.h instead of doing the forward declaration of struct
> XLogRecData; and in xlog_internal.h, instead of
>
> - * XLogRecord is defined in xlog.h, but we avoid #including that to keep
> + * XLogRecord is defined in xlogrecord.h, but we avoid #including that to keep
>
> I would just suggest to #include xlogrecord.h, since it should just
> work to include that file from frontend programs. It didn't work for
> xlog.h because that one #includes fmgr.h IIRC and that one causes Datum
> to appear, which makes compilation blow up. Shouldn't be the case here.
Alvaro got faster than me... I was just looking at the first patch and
+1 on those comments. It is worth noting that the first patch, as it
does only a large refactoring, does not impact performance or size of
WAL records. Btw, a declaration of access/xlog.h in fd.c is forgotten.
Also, if somebody is interested in running the test suite, there are
comments on how to do it at the top of compare.sql.
--
Michael