Make pg_interpret_timezone_abbrev() check sp->defaulttype too.
authorTom Lane <tgl@sss.pgh.pa.us>
Thu, 16 Jan 2025 17:43:03 +0000 (12:43 -0500)
committerTom Lane <tgl@sss.pgh.pa.us>
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++)
    {