Re: walprotocol.h vs frontends

On Mon, Aug 15, 2011 at 16:53, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> Magnus Hagander <magnus@hagander.net> writes:
>> On Mon, Aug 15, 2011 at 16:20, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>>> However, for a narrow fix, I could see moving the data type definition
>>> to someplace with fewer dependencies.  Perhaps split it into a separate
>>> file timestamp_type.h, or something like that.
>
>> Yes, that seems to fix the problem of timestamptz. See the attached
>> patch - seems ok?
>
> Don't think you should expose fsec_t, nor most of those macros.  The
> foo_per_bar values are just namespace clutter.

Hmm, ok. I just went for what seemed like a reasonable subset. I do
also need the SECS_PER_DAY and those constants in order to be able to
convert the timestamp value. That led me to include the other
foo_per_bar so they are all in one place - seems wrong to have them
split up.



>> I also ran into a similar problem with some WAL macro definitions that
>> are in xlog_internal.h. I've moved them to xlogdefs.h in the attached
>> xlog.diff file. Does that seem ok as well, or should I move them
>> somewhere else?
>
> I don't like the idea of exposing those to frontends, either.  What do
> you actually *need* out of that, and why?

PrevLogSeg - which also brings in XLogSegsPerFile.
XLogFileName
XLogFileSize - which brings in XLogSegsPerFile and XLogSegSize

PrevLogSeg should be self explaining, as should xlogfilename.

XLogFileSize is required by XLByteAdvance() which is already in
xlogdefs.h - so xlogdefs.h actually has a hidden dependency on
xlog_internal.h here today.

I can certainly separate those out, but it seemed more clean to move
the whole block they were in.

--
 Magnus Hagander
 Me: http://www.hagander.net/
 Work: http://www.redpill-linpro.com/