Re: Tracking timezone abbreviation removals in the IANA tz database - Mailing list pgsql-hackers

I wrote:
> So the idea I'm toying with (again, haven't tried to code this) is to say
> that *if* we can match the abbreviation to something in the referenced
> zone then we'll use that, but otherwise we fall back to treating the
> abbreviation as a macro for the zone name.

This turned out to require only fairly localized code changes.  So
I now propose that we leave the tznames files as-is and apply the
attached patch (plus documentation changes, which I've not done yet).
Any objections?

            regards, tom lane

diff --git a/src/backend/utils/adt/datetime.c b/src/backend/utils/adt/datetime.c
index 965c3b4..45ba7cd 100644
*** a/src/backend/utils/adt/datetime.c
--- b/src/backend/utils/adt/datetime.c
*************** static void AdjustFractDays(double frac,
*** 56,63 ****
                  int scale);
  static int DetermineTimeZoneOffsetInternal(struct pg_tm * tm, pg_tz *tzp,
                                  pg_time_t *tp);
! static int DetermineTimeZoneAbbrevOffsetInternal(pg_time_t t, const char *abbr,
!                                       pg_tz *tzp, int *isdst);
  static pg_tz *FetchDynamicTimeZone(TimeZoneAbbrevTable *tbl, const datetkn *tp);


--- 56,64 ----
                  int scale);
  static int DetermineTimeZoneOffsetInternal(struct pg_tm * tm, pg_tz *tzp,
                                  pg_time_t *tp);
! static bool DetermineTimeZoneAbbrevOffsetInternal(pg_time_t t,
!                                       const char *abbr, pg_tz *tzp,
!                                       int *offset, int *isdst);
  static pg_tz *FetchDynamicTimeZone(TimeZoneAbbrevTable *tbl, const datetkn *tp);


*************** overflow:
*** 1689,1707 ****
   * This differs from the behavior of DetermineTimeZoneOffset() in that a
   * standard-time or daylight-time abbreviation forces use of the corresponding
   * GMT offset even when the zone was then in DS or standard time respectively.
   */
  int
  DetermineTimeZoneAbbrevOffset(struct pg_tm * tm, const char *abbr, pg_tz *tzp)
  {
      pg_time_t    t;

      /*
       * Compute the UTC time we want to probe at.  (In event of overflow, we'll
       * probe at the epoch, which is a bit random but probably doesn't matter.)
       */
!     (void) DetermineTimeZoneOffsetInternal(tm, tzp, &t);

!     return DetermineTimeZoneAbbrevOffsetInternal(t, abbr, tzp, &tm->tm_isdst);
  }


--- 1690,1729 ----
   * This differs from the behavior of DetermineTimeZoneOffset() in that a
   * standard-time or daylight-time abbreviation forces use of the corresponding
   * GMT offset even when the zone was then in DS or standard time respectively.
+  * (However, that happens only if we can match the given abbreviation to some
+  * abbreviation that appears in the IANA timezone data.  Otherwise, we fall
+  * back to doing DetermineTimeZoneOffset().)
   */
  int
  DetermineTimeZoneAbbrevOffset(struct pg_tm * tm, const char *abbr, pg_tz *tzp)
  {
      pg_time_t    t;
+     int            zone_offset;
+     int            abbr_offset;
+     int            abbr_isdst;

      /*
       * Compute the UTC time we want to probe at.  (In event of overflow, we'll
       * probe at the epoch, which is a bit random but probably doesn't matter.)
       */
!     zone_offset = DetermineTimeZoneOffsetInternal(tm, tzp, &t);

!     /*
!      * Try to match the abbreviation to something in the zone definition.
!      */
!     if (DetermineTimeZoneAbbrevOffsetInternal(t, abbr, tzp,
!                                               &abbr_offset, &abbr_isdst))
!     {
!         /* Success, so use the abbrev-specific answers. */
!         tm->tm_isdst = abbr_isdst;
!         return abbr_offset;
!     }
!
!     /*
!      * No match, so use the answers we already got from
!      * DetermineTimeZoneOffsetInternal.
!      */
!     return zone_offset;
  }


*************** DetermineTimeZoneAbbrevOffsetTS(Timestam
*** 1715,1733 ****
                                  pg_tz *tzp, int *isdst)
  {
      pg_time_t    t = timestamptz_to_time_t(ts);

!     return DetermineTimeZoneAbbrevOffsetInternal(t, abbr, tzp, isdst);
  }


  /* DetermineTimeZoneAbbrevOffsetInternal()
   *
   * Workhorse for above two functions: work from a pg_time_t probe instant.
!  * DST status is returned into *isdst.
   */
! static int
! DetermineTimeZoneAbbrevOffsetInternal(pg_time_t t, const char *abbr,
!                                       pg_tz *tzp, int *isdst)
  {
      char        upabbr[TZ_STRLEN_MAX + 1];
      unsigned char *p;
--- 1737,1777 ----
                                  pg_tz *tzp, int *isdst)
  {
      pg_time_t    t = timestamptz_to_time_t(ts);
+     int            zone_offset;
+     int            abbr_offset;
+     int            tz;
+     struct pg_tm tm;
+     fsec_t        fsec;

!     /*
!      * If the abbrev matches anything in the zone data, this is pretty easy.
!      */
!     if (DetermineTimeZoneAbbrevOffsetInternal(t, abbr, tzp,
!                                               &abbr_offset, isdst))
!         return abbr_offset;
!
!     /*
!      * Else, break down the timestamp so we can use DetermineTimeZoneOffset.
!      */
!     if (timestamp2tm(ts, &tz, &tm, &fsec, NULL, tzp) != 0)
!         ereport(ERROR,
!                 (errcode(ERRCODE_DATETIME_VALUE_OUT_OF_RANGE),
!                  errmsg("timestamp out of range")));
!
!     zone_offset = DetermineTimeZoneOffset(&tm, tzp);
!     *isdst = tm.tm_isdst;
!     return zone_offset;
  }


  /* DetermineTimeZoneAbbrevOffsetInternal()
   *
   * Workhorse for above two functions: work from a pg_time_t probe instant.
!  * On success, return GMT offset and DST status into *offset and *isdst.
   */
! static bool
! DetermineTimeZoneAbbrevOffsetInternal(pg_time_t t, const char *abbr, pg_tz *tzp,
!                                       int *offset, int *isdst)
  {
      char        upabbr[TZ_STRLEN_MAX + 1];
      unsigned char *p;
*************** DetermineTimeZoneAbbrevOffsetInternal(pg
*** 1739,1756 ****
          *p = pg_toupper(*p);

      /* Look up the abbrev's meaning at this time in this zone */
!     if (!pg_interpret_timezone_abbrev(upabbr,
!                                       &t,
!                                       &gmtoff,
!                                       isdst,
!                                       tzp))
!         ereport(ERROR,
!                 (errcode(ERRCODE_CONFIG_FILE_ERROR),
!                  errmsg("time zone abbreviation \"%s\" is not used in time zone \"%s\"",
!                         abbr, pg_get_timezone_name(tzp))));
!
!     /* Change sign to agree with DetermineTimeZoneOffset() */
!     return (int) -gmtoff;
  }


--- 1783,1799 ----
          *p = pg_toupper(*p);

      /* Look up the abbrev's meaning at this time in this zone */
!     if (pg_interpret_timezone_abbrev(upabbr,
!                                      &t,
!                                      &gmtoff,
!                                      isdst,
!                                      tzp))
!     {
!         /* Change sign to agree with DetermineTimeZoneOffset() */
!         *offset = (int) -gmtoff;
!         return true;
!     }
!     return false;
  }


diff --git a/src/test/regress/expected/timestamptz.out b/src/test/regress/expected/timestamptz.out
index 67f26db..2bfc13a 100644
*** a/src/test/regress/expected/timestamptz.out
--- b/src/test/regress/expected/timestamptz.out
*************** SELECT '2007-12-09 07:30:00 UTC'::timest
*** 2603,2605 ****
--- 2603,2625 ----
   Sun Dec 09 03:00:00 2007
  (1 row)

+ --
+ -- Test that the pg_timezone_names and pg_timezone_abbrevs views are
+ -- more-or-less working.  We can't test their contents in any great detail
+ -- without the outputs changing anytime IANA updates the underlying data,
+ -- but it seems reasonable to expect at least one entry per major meridian.
+ -- (At the time of writing, the actual counts are around 38 because of
+ -- zones using fractional GMT offsets, so this is a pretty loose test.)
+ --
+ select count(distinct utc_offset) >= 24 as ok from pg_timezone_names;
+  ok
+ ----
+  t
+ (1 row)
+
+ select count(distinct utc_offset) >= 24 as ok from pg_timezone_abbrevs;
+  ok
+ ----
+  t
+ (1 row)
+
diff --git a/src/test/regress/sql/timestamptz.sql b/src/test/regress/sql/timestamptz.sql
index c023095..ce9d1c2 100644
*** a/src/test/regress/sql/timestamptz.sql
--- b/src/test/regress/sql/timestamptz.sql
*************** SELECT '2007-12-09 07:00:00 UTC'::timest
*** 468,470 ****
--- 468,481 ----
  SELECT '2007-12-09 07:00:01 UTC'::timestamptz AT TIME ZONE 'VET';
  SELECT '2007-12-09 07:29:59 UTC'::timestamptz AT TIME ZONE 'VET';
  SELECT '2007-12-09 07:30:00 UTC'::timestamptz AT TIME ZONE 'VET';
+
+ --
+ -- Test that the pg_timezone_names and pg_timezone_abbrevs views are
+ -- more-or-less working.  We can't test their contents in any great detail
+ -- without the outputs changing anytime IANA updates the underlying data,
+ -- but it seems reasonable to expect at least one entry per major meridian.
+ -- (At the time of writing, the actual counts are around 38 because of
+ -- zones using fractional GMT offsets, so this is a pretty loose test.)
+ --
+ select count(distinct utc_offset) >= 24 as ok from pg_timezone_names;
+ select count(distinct utc_offset) >= 24 as ok from pg_timezone_abbrevs;