Make pg_interpret_timezone_abbrev() check sp->defaulttype too.
authorTom Lane <[email protected]>
Thu, 16 Jan 2025 17:43:03 +0000 (12:43 -0500)
committerTom Lane <[email protected]>
Thu, 16 Jan 2025 17:43:03 +0000 (12:43 -0500)
This omission caused it to not recognize the furthest-back zone
abbreviation when working with timezone data compiled with relatively
recent zic (2018f or newer).  Older versions of zic produced a dummy
DST transition at the Big Bang, so that the oldest abbreviation could
always be found in the sp->types[] array; but newer versions don't do
that, so that we must examine defaulttype as well as the types[] array
to be sure of seeing all the abbreviations.

While this has been broken for six or so years, we'd managed not
to notice for two reasons: (1) many platforms are still using
ancient zic for compatibility reasons, so that the issue did not
manifest in builds using --with-system-tzdata; (2) the oldest
zone abbreviation is almost always "LMT", which we weren't
supporting anyway (but an upcoming patch will accept that).

While at it, update pg_next_dst_boundary() to use sp->defaulttype
as the time type for non-DST zones and times before the oldest
DST transition.  The existing code there predates upstream's
invention of the sp->defaulttype field, and its heuristic for
finding the oldest time type has now been subsumed into the
code that sets sp->defaulttype.

Possibly this should be back-patched, but I'm not currently aware
of any visible consequences of this bug in released branches.

Per report from Aleksander Alekseev and additional investigation.

Discussion: https://postgr.es/m/CAJ7c6TOATjJqvhnYsui0=CO5XFMF4dvTGH+skzB--jNhqSQu5g@mail.gmail.com

src/timezone/localtime.c

index 0bc160ea7d79fb73eabfe45bc1a2d4e658857543..21516c65082ed84571c3c5458417261f0223fbc1 100644 (file)
@@ -1624,15 +1624,8 @@ pg_next_dst_boundary(const pg_time_t *timep,
        sp = &tz->state;
        if (sp->timecnt == 0)
        {
-               /* non-DST zone, use lowest-numbered standard type */
-               i = 0;
-               while (sp->ttis[i].tt_isdst)
-                       if (++i >= sp->typecnt)
-                       {
-                               i = 0;
-                               break;
-                       }
-               ttisp = &sp->ttis[i];
+               /* non-DST zone, use the defaulttype */
+               ttisp = &sp->ttis[sp->defaulttype];
                *before_gmtoff = ttisp->tt_utoff;
                *before_isdst = ttisp->tt_isdst;
                return 0;
@@ -1692,15 +1685,8 @@ pg_next_dst_boundary(const pg_time_t *timep,
        }
        if (t < sp->ats[0])
        {
-               /* For "before", use lowest-numbered standard type */
-               i = 0;
-               while (sp->ttis[i].tt_isdst)
-                       if (++i >= sp->typecnt)
-                       {
-                               i = 0;
-                               break;
-                       }
-               ttisp = &sp->ttis[i];
+               /* For "before", use the defaulttype */
+               ttisp = &sp->ttis[sp->defaulttype];
                *before_gmtoff = ttisp->tt_utoff;
                *before_isdst = ttisp->tt_isdst;
                *boundary = sp->ats[0];
@@ -1793,7 +1779,9 @@ pg_interpret_timezone_abbrev(const char *abbrev,
         * abbreviation should get us what we want, since extrapolation would just
         * be repeating the newest or oldest meanings.
         *
-        * Use binary search to locate the first transition > cutoff time.
+        * Use binary search to locate the first transition > cutoff time.  (Note
+        * that sp->timecnt could be zero, in which case this loop does nothing
+        * and only the defaulttype entry will be checked.)
         */
        {
                int                     lo = 0;
@@ -1827,7 +1815,19 @@ pg_interpret_timezone_abbrev(const char *abbrev,
        }
 
        /*
-        * Not there, so scan forwards to find the first one after.
+        * Not found yet; check the defaulttype, which is notionally the era
+        * before any of the entries in sp->types[].
+        */
+       ttisp = &sp->ttis[sp->defaulttype];
+       if (ttisp->tt_desigidx == abbrind)
+       {
+               *gmtoff = ttisp->tt_utoff;
+               *isdst = ttisp->tt_isdst;
+               return true;
+       }
+
+       /*
+        * Not there, so scan forwards to find the first one after the cutoff.
         */
        for (i = cutoff; i < sp->timecnt; i++)
        {