Re: [PATCH] Add TimeOffset and DateOffset typedefs

On Thu, Mar 20, 2008 at 6:41 PM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> Warren Turkal <turkal(at)google(dot)com> writes:
> > I added TimeOffset and DateOffset typedefs to get rid of the instances
> > using the HAVE_INT64_TIMESTAMP define being used to determine the
> > types of variables or functions in timestamp.c.
>
> Applied with minor revisions. I dropped DateOffset since it didn't seem
> to be pulling its weight --- there was only one use and that was
> probably better declared as TimeOffset anyway. We can always add it
> later if we really do need it. Also I knocked over one or two other
> places (in files beyond the ones you touched) where there was a
> conditional declaration that could be eliminated.
>
> There sure are a lot of #ifdef HAVE_INT64_TIMESTAMP's left, though,
> aren't there :-(. It looked like the next thing to think about was
> how to unify the scale-dependent calculations. We could make some
> headway by defining a conversion constant that was either 1000000
> or 1.0, but I'm worried about whether the C compiler is always smart
> enough to optimize away a floating point multiplication or division
> by 1.0 (there are at least some contexts where it *shouldn't* do
> that, I think). Might be better to make macros that either
> multiply/divide by 1000000 or do nothing.
>
> BTW, not sure if you're aware of this, but pgindent tends to add
> and subtract blank lines around #if/#else/#endif commands in weird,
> inconsistent ways. If the vertical spacing seems a bit odd after
> you've removed a conditional, feel free to fix it. I fixed a number
> of places like that in this patch.

Thanks for the info. I will look over the conversions. I have had them
in the back of my mind for a little while.

wt