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/
