Обсуждение: [PATCH] Add TimeOffset and DateOffset typedefs

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.
---src/backend/utils/adt/timestamp.c |   77 +++++++-----------------------------src/include/utils/timestamp.h     |
4++2 files changed, 19 insertions(+), 62 deletions(-)
 

diff --git a/src/backend/utils/adt/timestamp.c b/src/backend/utils/adt/timestamp.c
index 9b90873..4759e22 100644
--- a/src/backend/utils/adt/timestamp.c
+++ b/src/backend/utils/adt/timestamp.c
@@ -44,11 +44,7 @@TimestampTz PgStartTime;
-#ifdef HAVE_INT64_TIMESTAMP
-static int64 time2t(const int hour, const int min, const int sec, const fsec_t fsec);
-#else
-static double time2t(const int hour, const int min, const int sec, const fsec_t fsec);
-#endif
+static TimeOffset time2t(const int hour, const int min, const int sec, const fsec_t fsec);static int
EncodeSpecialTimestamp(Timestampdt, char *str);static Timestamp dt2local(Timestamp dt, int timezone);static void
AdjustTimestampForTypmod(Timestamp*time, int32 typmod);
 
@@ -977,11 +973,7 @@ AdjustIntervalForTypmod(Interval *interval, int32 typmod)        }        else if (range ==
INTERVAL_MASK(MINUTE))       {
 
-#ifdef HAVE_INT64_TIMESTAMP
-            int64        hour;
-#else
-            double        hour;
-#endif
+            TimeOffset    hour;            interval->month = 0;            interval->day = 0;
@@ -998,11 +990,7 @@ AdjustIntervalForTypmod(Interval *interval, int32 typmod)        }        else if (range ==
INTERVAL_MASK(SECOND))       {
 
-#ifdef HAVE_INT64_TIMESTAMP
-            int64        minute;
-#else
-            double        minute;
-#endif
+            TimeOffset    minute;            interval->month = 0;            interval->day = 0;
@@ -1076,11 +1064,7 @@ AdjustIntervalForTypmod(Interval *interval, int32 typmod)        else if (range ==
(INTERVAL_MASK(MINUTE)|                           INTERVAL_MASK(SECOND)))        {
 
-#ifdef HAVE_INT64_TIMESTAMP
-            int64        hour;
-#else
-            double        hour;
-#endif
+            TimeOffset    hour;            interval->month = 0;            interval->day = 0;
@@ -1342,11 +1326,7 @@ timestamptz_to_str(TimestampTz t)voiddt2time(Timestamp jd, int *hour, int *min, int *sec, fsec_t
*fsec){
-#ifdef HAVE_INT64_TIMESTAMP
-    int64        time;
-#else
-    double        time;
-#endif
+    TimeOffset    time;    time = jd;
@@ -1547,13 +1527,8 @@ recalc_t:inttm2timestamp(struct pg_tm * tm, fsec_t fsec, int *tzp, Timestamp *result){
-#ifdef HAVE_INT64_TIMESTAMP
-    int            date;
-    int64        time;
-#else
-    double        date,
-                time;
-#endif
+    DateOffset    date;
+    TimeOffset    time;    /* Julian day routines are not correct for negative Julian days */    if
(!IS_VALID_JULIAN(tm->tm_year,tm->tm_mon, tm->tm_mday))
 
@@ -1596,13 +1571,8 @@ tm2timestamp(struct pg_tm * tm, fsec_t fsec, int *tzp, Timestamp *result)intinterval2tm(Interval
span,struct pg_tm * tm, fsec_t *fsec){
 
-#ifdef HAVE_INT64_TIMESTAMP
-    int64        time;
-    int64        tfrac;
-#else
-    double        time;
-    double        tfrac;
-#endif
+    TimeOffset    time;
+    TimeOffset    tfrac;    tm->tm_year = span.month / MONTHS_PER_YEAR;    tm->tm_mon = span.month % MONTHS_PER_YEAR;
@@ -1658,19 +1628,15 @@ tm2interval(struct pg_tm * tm, fsec_t fsec, Interval *span)    return 0;}
-#ifdef HAVE_INT64_TIMESTAMP
-static int64
+static TimeOffsettime2t(const int hour, const int min, const int sec, const fsec_t fsec){
+#ifdef HAVE_INT64_TIMESTAMP    return (((((hour * MINS_PER_HOUR) + min) * SECS_PER_MINUTE) + sec) * USECS_PER_SEC) +
fsec;
-}    /* time2t() */#else
-static double
-time2t(const int hour, const int min, const int sec, const fsec_t fsec)
-{    return (((hour * MINS_PER_HOUR) + min) * SECS_PER_MINUTE) + sec + fsec;
-}    /* time2t() */#endif
+}    /* time2t() */static Timestampdt2local(Timestamp dt, int tz)
@@ -2042,13 +2008,8 @@ timestamptz_cmp_timestamp(PG_FUNCTION_ARGS)static intinterval_cmp_internal(Interval *interval1,
Interval*interval2){
 
-#ifdef HAVE_INT64_TIMESTAMP
-    int64        span1,
+    TimeOffset    span1,                span2;
-#else
-    double        span1,
-                span2;
-#endif    span1 = interval1->time;    span2 = interval2->time;
@@ -2388,11 +2349,7 @@ interval_justify_interval(PG_FUNCTION_ARGS)    Interval   *span = PG_GETARG_INTERVAL_P(0);
Interval  *result;
 
-#ifdef HAVE_INT64_TIMESTAMP
-    int64        wholeday;
-#else
-    double        wholeday;
-#endif
+    TimeOffset    wholeday;    int32        wholemonth;    result = (Interval *) palloc(sizeof(Interval));
@@ -2460,11 +2417,7 @@ interval_justify_hours(PG_FUNCTION_ARGS)    Interval   *span = PG_GETARG_INTERVAL_P(0);
Interval  *result;
 
-#ifdef HAVE_INT64_TIMESTAMP
-    int64        wholeday;
-#else
-    double        wholeday;
-#endif
+    TimeOffset    wholeday;    result = (Interval *) palloc(sizeof(Interval));    result->month = span->month;
diff --git a/src/include/utils/timestamp.h b/src/include/utils/timestamp.h
index 34e6644..63e033c 100644
--- a/src/include/utils/timestamp.h
+++ b/src/include/utils/timestamp.h
@@ -36,9 +36,13 @@#ifdef HAVE_INT64_TIMESTAMPtypedef int64 Timestamp;typedef int64 TimestampTz;
+typedef int64 TimeOffset;
+typedef int   DateOffset;#elsetypedef double Timestamp;typedef double TimestampTz;
+typedef double TimeOffset;
+typedef double DateOffset;#endiftypedef struct
-- 
1.5.2.5

I have removed all instances of using HAVE_INT64_TIMESTAMP to determine the
type of a variable in files depending on timestamp.h.
---src/backend/utils/adt/nabstime.c |    6 +-----1 files changed, 1 insertions(+), 5 deletions(-)

diff --git a/src/backend/utils/adt/nabstime.c b/src/backend/utils/adt/nabstime.c
index 992f7ab..5a662f3 100644
--- a/src/backend/utils/adt/nabstime.c
+++ b/src/backend/utils/adt/nabstime.c
@@ -832,11 +832,7 @@ interval_reltime(PG_FUNCTION_ARGS)                month,                day;
-#ifdef HAVE_INT64_TIMESTAMP
-    int64        span;
-#else
-    double        span;
-#endif
+    TimeOffset        span;    year = interval->month / MONTHS_PER_YEAR;    month = interval->month %
MONTHS_PER_YEAR;
-- 
1.5.2.5

Warren Turkal <turkal@google.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.
        regards, tom lane

On Thu, Mar 20, 2008 at 6:41 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> Warren Turkal <turkal@google.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