Обсуждение: More tzdb fun: POSIXRULES is being deprecated upstream

Paul Eggert, in https://mm.icann.org/pipermail/tz/2019-June/028172.html:
> zic’s -p option was intended as a transition from historical
> System V code that treated TZ="XXXnYYY" as meaning US
> daylight-saving rules in a time zone n hours west of UT,
> with XXX abbreviating standard time and YYY abbreviating DST.
> zic -p allows the tzdata installer to specify (say)
> Europe/Brussels's rules instead of US rules.  This behavior
> is not well documented and often fails in practice; for example it
> does not work with current glibc for contemporary timestamps, and
> it does not work in tzdb itself for timestamps after 2037.
> So, document it as being obsolete, with the intent that it
> will be removed in a future version.  This change does not
> affect behavior of the default installation.

As he says, this doesn't work for post-2038 dates:

regression=# set timezone = 'FOO5BAR';
SET
regression=# select now();
              now
-------------------------------
 2019-07-04 11:55:46.905382-04
(1 row)

regression=# select timeofday();
              timeofday
-------------------------------------
 Thu Jul 04 11:56:14.102770 2019 BAR
(1 row)

regression=# select '2020-07-04'::timestamptz;
      timestamptz
------------------------
 2020-07-04 00:00:00-04
(1 row)

regression=# select '2040-07-04'::timestamptz;
      timestamptz
------------------------
 2040-07-04 00:00:00-05                  <<-- should be -04
(1 row)

and this note makes it clear that the IANA crew aren't planning on fixing
that.  It does work if you write a full POSIX-style DST specification:

regression=# set timezone = 'FOO5BAR,M3.2.0,M11.1.0';
SET
regression=# select '2040-07-04'::timestamptz;
      timestamptz
------------------------
 2040-07-04 00:00:00-04
(1 row)

so I think what Eggert has in mind here is that they'll remove the
TZDEFRULES-loading logic and always fall back to TZDEFRULESTRING when
presented with a POSIX-style zone spec that lacks explicit transition
date rules.

So, what if anything should we do about this?  We do document posixrules,
very explicitly, see datatype.sgml around line 2460:

        When a daylight-savings zone abbreviation is present,
        it is assumed to be used
        according to the same daylight-savings transition rules used in the
        IANA time zone database's <filename>posixrules</filename> entry.
        In a standard <productname>PostgreSQL</productname> installation,
        <filename>posixrules</filename> is the same as <literal>US/Eastern</literal>, so
        that POSIX-style time zone specifications follow USA daylight-savings
        rules.  If needed, you can adjust this behavior by replacing the
        <filename>posixrules</filename> file.

One option is to do nothing until the IANA code actually changes,
but as 2038 gets closer, people are more likely to start noticing
that this "feature" doesn't work as one would expect.

We could get out front of the problem and remove the TZDEFRULES-loading
logic ourselves.  That would be a bit of a maintenance hazard, but perhaps
not too awful, because we already deviate from the IANA code in that area
(we have our own ideas about when/whether to try to load TZDEFRULES).

I don't think we'd want to change this behavior in the back branches,
but it might be OK to do it as a HEAD change.  I think I'd rather do
it like that than be forced into playing catchup when the IANA code
does change.

A more aggressive idea would be to stop supporting POSIX-style timezone
specs altogether, but I'm not sure I like that answer.  Even if we could
get away with it from a users'-eye standpoint, I think we have some
internal dependencies on being able to use such specifications.

            regards, tom lane

Last year I wrote:
> Paul Eggert, in https://mm.icann.org/pipermail/tz/2019-June/028172.html:
>> zic’s -p option was intended as a transition from historical
>> System V code that treated TZ="XXXnYYY" as meaning US
>> daylight-saving rules in a time zone n hours west of UT,
>> with XXX abbreviating standard time and YYY abbreviating DST.
>> zic -p allows the tzdata installer to specify (say)
>> Europe/Brussels's rules instead of US rules.  This behavior
>> is not well documented and often fails in practice; for example it
>> does not work with current glibc for contemporary timestamps, and
>> it does not work in tzdb itself for timestamps after 2037.
>> So, document it as being obsolete, with the intent that it
>> will be removed in a future version.  This change does not
>> affect behavior of the default installation.

Well, we ignored this for a year, but it's about to become unavoidable:
http://mm.icann.org/pipermail/tz/2020-June/029093.html
IANA upstream is changing things so that by default there will not be
any "posixrules" file in the tz database.

That wouldn't directly affect our builds, since we don't use their
Makefile anyway.  But it will affect installations that use
--with-system-tzdata, which I believe is most vendor-packaged
Postgres installations.

It's possible that most or even all tzdata packagers will ignore
the change and continue to ship a posixrules file, for backwards
compatibility's sake.  But I doubt we should bet that way.
glibc-based distros, in particular, would have little motivation to
do so.  We should expect that, starting probably this fall, there
will be installations with no posixrules file.

The minimum thing that we have to do, I'd say, is to change the
documentation to explain what happens if there's no posixrules file.
However, in view of the fact that the posixrules feature doesn't work
past 2037 and isn't going to be fixed, maybe we should just nuke it
now rather than waiting for our hand to be forced.  I'm not sure that
I've ever heard of anyone replacing the posixrules file anyway.
(The fallback case is actually better in that it works for dates past
2037; it's worse only in that you can't configure it.)

I would definitely be in favor of "nuke it now" with respect to HEAD.
It's a bit more debatable for the back branches.  However, all branches
are going to be equally exposed to updated system tzdata trees, so
we've typically felt that changes in the tz-related code should be
back-patched.

Thoughts?

            regards, tom lane

I wrote:
> The minimum thing that we have to do, I'd say, is to change the
> documentation to explain what happens if there's no posixrules file.

Here's a proposed patch to do that.  To explain this, we more or less
have to fully document the POSIX timezone string format (otherwise
nobody's gonna understand what "M3.2.0,M11.1.0" means).  That's something
we've glossed over for many years, and I still feel like it's not
something to explain in-line in section 8.5.3, so I shoved all the gory
details into a new section in Appendix B.  To be clear, nothing here is
new behavior, it was just undocumented before.

            regards, tom lane

diff --git a/doc/src/sgml/datatype.sgml b/doc/src/sgml/datatype.sgml
index 3df189ad85..ca61439501 100644
--- a/doc/src/sgml/datatype.sgml
+++ b/doc/src/sgml/datatype.sgml
@@ -2492,25 +2492,10 @@ TIMESTAMP WITH TIME ZONE '2004-10-19 10:23:54+02'
        <para>
         In addition to the timezone names and abbreviations,
         <productname>PostgreSQL</productname> will accept POSIX-style time zone
-        specifications of the form <replaceable>STD</replaceable><replaceable>offset</replaceable> or
-        <replaceable>STD</replaceable><replaceable>offset</replaceable><replaceable>DST</replaceable>, where
-        <replaceable>STD</replaceable> is a zone abbreviation, <replaceable>offset</replaceable> is a
-        numeric offset in hours west from UTC, and <replaceable>DST</replaceable> is an
-        optional daylight-savings zone abbreviation, assumed to stand for one
-        hour ahead of the given offset. For example, if <literal>EST5EDT</literal>
-        were not already a recognized zone name, it would be accepted and would
-        be functionally equivalent to United States East Coast time.  In this
-        syntax, a zone abbreviation can be a string of letters, or an
-        arbitrary string surrounded by angle brackets (<literal><></literal>).
-        When a daylight-savings zone abbreviation is present,
-        it is assumed to be used
-        according to the same daylight-savings transition rules used in the
-        IANA time zone database's <filename>posixrules</filename> entry.
-        In a standard <productname>PostgreSQL</productname> installation,
-        <filename>posixrules</filename> is the same as <literal>US/Eastern</literal>, so
-        that POSIX-style time zone specifications follow USA daylight-savings
-        rules.  If needed, you can adjust this behavior by replacing the
-        <filename>posixrules</filename> file.
+        specifications, as described in
+        <xref linkend="datetime-posix-timezone-specs"/>.  This option is not
+        normally preferable to using a named time zone, but it may be
+        necessary if no suitable IANA time zone entry is available.
        </para>
       </listitem>
      </itemizedlist>
@@ -2537,19 +2522,6 @@ TIMESTAMP WITH TIME ZONE '2004-10-19 10:23:54+02'
      above, this is not necessarily the same as local civil time on that date.
     </para>

-    <para>
-     One should be wary that the POSIX-style time zone feature can
-     lead to silently accepting bogus input, since there is no check on the
-     reasonableness of the zone abbreviations.  For example, <literal>SET
-     TIMEZONE TO FOOBAR0</literal> will work, leaving the system effectively using
-     a rather peculiar abbreviation for UTC.
-     Another issue to keep in mind is that in POSIX time zone names,
-     positive offsets are used for locations <emphasis>west</emphasis> of Greenwich.
-     Everywhere else, <productname>PostgreSQL</productname> follows the
-     ISO-8601 convention that positive timezone offsets are <emphasis>east</emphasis>
-     of Greenwich.
-    </para>
-
     <para>
      In all cases, timezone names and abbreviations are recognized
      case-insensitively.  (This is a change from <productname>PostgreSQL</productname>
diff --git a/doc/src/sgml/datetime.sgml b/doc/src/sgml/datetime.sgml
index 7cce826e2d..fb210b377b 100644
--- a/doc/src/sgml/datetime.sgml
+++ b/doc/src/sgml/datetime.sgml
@@ -555,6 +555,210 @@

   </sect1>

+  <sect1 id="datetime-posix-timezone-specs">
+  <title><acronym>POSIX</acronym> Time Zone Specifications</title>
+
+  <indexterm zone="datetime-posix-timezone-specs">
+   <primary>time zone</primary>
+   <secondary><acronym>POSIX</acronym>-style specification</secondary>
+  </indexterm>
+
+  <para>
+   <acronym>PostgreSQL</acronym> can accept time zone specifications that
+   are written according to the <acronym>POSIX</acronym> standard's rules
+   for the <varname>TZ</varname> environment
+   variable.  <acronym>POSIX</acronym> time zone specifications are
+   inadequate to deal with the complexity of real-world time zone history,
+   but there are sometimes reasons to use them.
+  </para>
+
+  <para>
+   A POSIX time zone specification has the form
+<synopsis>
+<replaceable>STD</replaceable> <replaceable>offset</replaceable> <optional> <replaceable>DST</replaceable> <optional>
<replaceable>dstoffset</replaceable></optional> <optional> , <replaceable>rule</replaceable> </optional> </optional> 
+</synopsis>
+   (For readability, we show spaces between the fields, but spaces should
+   not be used in practice.)  The fields are:
+   <itemizedlist>
+    <listitem>
+     <para>
+      <replaceable>STD</replaceable> is the zone abbreviation to be used
+      for standard time.
+     </para>
+    </listitem>
+    <listitem>
+     <para>
+      <replaceable>offset</replaceable> is the zone's standard-time offset
+      from UTC.
+     </para>
+    </listitem>
+    <listitem>
+     <para>
+      <replaceable>DST</replaceable> is the zone abbreviation to be used
+      for daylight-savings time.  If this field and the following ones are
+      omitted, the zone uses a fixed UTC offset with no daylight-savings
+      rule.
+     </para>
+    </listitem>
+    <listitem>
+     <para>
+      <replaceable>dstoffset</replaceable> is the daylight-savings offset
+      from UTC.  This field is typically omitted, since it defaults to one
+      hour less than the standard-time <replaceable>offset</replaceable>,
+      which is usually the right thing.
+     </para>
+    </listitem>
+    <listitem>
+     <para>
+      <replaceable>rule</replaceable> defines the rule for when daylight
+      savings is in effect, as described below.
+     </para>
+    </listitem>
+   </itemizedlist>
+  </para>
+
+  <para>
+   In this syntax, a zone abbreviation can be a string of letters, or an
+   arbitrary string surrounded by angle brackets
+   (<literal><></literal>).  Note that the zone abbreviations given
+   here are only used for output, and even then only in some timestamp
+   output formats.  The zone abbreviations recognized in timestamp input
+   are determined as explained in <xref linkend="datetime-config-files"/>.
+  </para>
+
+  <para>
+   The offset fields specify the hours, and optionally minutes and seconds,
+   difference from UTC.  They have the format
+
<replaceable>hh</replaceable><optional><literal>:</literal><replaceable>mm</replaceable><optional><literal>:</literal><replaceable>ss</replaceable></optional></optional>
+   optionally with a leading sign (<literal>+</literal>
+   or <literal>-</literal>).  A positive sign is considered to define
+   zones <emphasis>west</emphasis> of Greenwich.  (Note that this is the
+   opposite of the ISO-8601 sign convention used elsewhere in
+   <acronym>PostgreSQL</acronym>.)
+  </para>
+
+  <para>
+   The daylight-savings transition <replaceable>rule</replaceable> has the
+   format
+<synopsis>
+<replaceable>dstdate</replaceable> <optional> <literal>/</literal> <replaceable>dsttime</replaceable> </optional>
<literal>,</literal><replaceable>stddate</replaceable> <optional> <literal>/</literal>
<replaceable>stdtime</replaceable></optional> 
+</synopsis>
+   (As before, spaces should not be included in practice.)
+   The <replaceable>dstdate</replaceable>
+   and <replaceable>dsttime</replaceable> fields define when daylight-savings
+   time starts, while <replaceable>stddate</replaceable>
+   and <replaceable>stdtime</replaceable> define when standard time
+   starts.  (In some cases, notably in zones south of the equator, the
+   former might be later in the year than the latter.)  The date fields
+   have one of these formats:
+   <variablelist>
+    <varlistentry>
+     <term><replaceable>n</replaceable></term>
+     <listitem>
+      <para>
+       A plain integer denotes a day of the year, counting from zero to
+       364, or to 365 in leap years.
+      </para>
+     </listitem>
+    </varlistentry>
+    <varlistentry>
+     <term><literal>J</literal><replaceable>n</replaceable></term>
+     <listitem>
+      <para>
+       In this form, <replaceable>n</replaceable> counts from 1 to 365,
+       and February 29 is not counted even if it is present.  (Thus, a
+       transition occurring on February 29 could not be specified this way.)
+      </para>
+     </listitem>
+    </varlistentry>
+    <varlistentry>
+
<term><literal>M</literal><replaceable>m</replaceable><literal>.</literal><replaceable>n</replaceable><literal>.</literal><replaceable>d</replaceable></term>
+     <listitem>
+      <para>
+       This form specifies a transition occurring on a particular day of
+       the week.  <replaceable>d</replaceable> is the weekday number from
+       0 to 6 (Sunday is 0).  <replaceable>m</replaceable> identifies the
+       month, from 1 to 12.  <replaceable>n</replaceable> specifies
+       the <replaceable>n</replaceable>'th occurrence of that weekday
+       within the month (1–4), or 5 meaning the last occurrence of
+       that weekday in the month (which could be the fourth or the fifth).
+      </para>
+     </listitem>
+    </varlistentry>
+   </variablelist>
+  </para>
+
+  <note>
+   <para>
+    The <literal>M</literal> format is sufficient to describe many common
+    daylight-savings transition rules; for
+    example <literal>M3.2.0</literal> means <quote>second Sunday in
+    March</quote>.  But note that none of these variants can deal with
+    daylight-savings law changes, so in practice the historical data
+    stored for named time zones (in the IANA time zone database) is
+    necessary to interpret past time stamps correctly.
+   </para>
+  </note>
+
+  <para>
+   The time fields in a transition rule have the same format as the offset
+   fields described previously, except that they cannot contain signs.
+   They define the current local time at which the change to the other
+   time occurs.  If omitted, they default to <literal>02:00:00</literal>.
+  </para>
+
+  <para>
+   If a daylight-savings abbreviation is given but the
+   transition <replaceable>rule</replaceable> field is omitted,
+   <productname>PostgreSQL</productname> attempts to determine the
+   transition times by consulting the <filename>posixrules</filename> file
+   in the IANA time zone database.  This file has the same format as a
+   full time zone entry, but only its transition times are used, not its
+   UTC offsets.  Typically, this file has the same contents as the
+   <literal>US/Eastern</literal> file, so that POSIX-style time zone
+   specifications follow USA daylight-savings rules.  If needed, you can
+   adjust this behavior by replacing the <filename>posixrules</filename>
+   file.
+  </para>
+
+  <para>
+   If the <filename>posixrules</filename> file is not present,
+   the fallback behavior is to use the
+   rule <literal>M3.2.0,M11.1.0</literal>, which corresponds to USA
+   practice as of 2020 (that is, spring forward on the second Sunday of
+   March, fall back on the first Sunday of November, both transitions
+   occurring at 2AM prevailing time).
+  </para>
+
+  <note>
+   <para>
+    The facility to consult a <filename>posixrules</filename> file has
+    been deprecated by IANA, and it is likely to go away in the future.
+    One bug in this feature, which is unlikely to be fixed before it
+    disappears, is that it fails to apply DST rules to dates after 2038.
+   </para>
+  </note>
+
+  <para>
+   As an example, <literal>CET-1CEST,M3.5.0,M10.5.0/3</literal> describes
+   current (as of 2020) timekeeping practice in Paris.  This specification
+   says that standard time has the abbreviation <literal>CET</literal> and
+   is one hour ahead of UTC; daylight savings time has the
+   abbreviation <literal>CEST</literal> and is implicitly two hours ahead
+   of UTC; daylight savings time begins on the last Sunday in March at 2AM
+   and ends on the last Sunday in October at 3AM.
+  </para>
+
+  <para>
+   One should be wary that it is easy to misspell a POSIX-style time zone
+   specification, since there is no check on the reasonableness of the
+   zone abbreviation(s).  For example, <literal>SET TIMEZONE TO
+   FOOBAR0</literal> will work, leaving the system effectively using a
+   rather peculiar abbreviation for UTC.
+  </para>
+
+  </sect1>
+
   <sect1 id="datetime-units-history">
   <title>History of Units</title>

On Thu, Jun 18, 2020 at 12:26 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:
> Here's a proposed patch to do that.  To explain this, we more or less
> have to fully document the POSIX timezone string format (otherwise
> nobody's gonna understand what "M3.2.0,M11.1.0" means).  That's something
> we've glossed over for many years, and I still feel like it's not
> something to explain in-line in section 8.5.3, so I shoved all the gory
> details into a new section in Appendix B.  To be clear, nothing here is
> new behavior, it was just undocumented before.

I'm glad you are proposing to document this, because the set of people
who had no idea what "M3.2.0,M11.1.0" means definitely included me.
It's a little confusing, though, that you documented it as Mm.n.d but
then in the text the order of explanation is d then m then n. Maybe
switch the text around so the order matches, or even use something
like Mmonth.occurrence.day.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

Robert Haas <robertmhaas@gmail.com> writes:
> It's a little confusing, though, that you documented it as Mm.n.d but
> then in the text the order of explanation is d then m then n. Maybe
> switch the text around so the order matches, or even use something
> like Mmonth.occurrence.day.

Yeah, I struggled with that text for a bit.  It doesn't seem to make sense
to explain that n means the n'th occurrence of a particular d value before
we've explained what d is, so explaining the fields in their syntactic
order seems like a loser.  But we could describe m first without that
problem.

Not sure about replacing the m/n/d notation --- that's straight out of
POSIX, so inventing our own terminology might just confuse people who
do know the spec.

            regards, tom lane

On Thu, Jun 18, 2020 at 1:05 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
> Robert Haas <robertmhaas@gmail.com> writes:
> > It's a little confusing, though, that you documented it as Mm.n.d but
> > then in the text the order of explanation is d then m then n. Maybe
> > switch the text around so the order matches, or even use something
> > like Mmonth.occurrence.day.
>
> Yeah, I struggled with that text for a bit.  It doesn't seem to make sense
> to explain that n means the n'th occurrence of a particular d value before
> we've explained what d is, so explaining the fields in their syntactic
> order seems like a loser.  But we could describe m first without that
> problem.

You could consider something along the lines of:

This form specifies a transition that always happens during the same
month and on the same day of the week. m identifies the month, from 1
to 12. n specifies the n'th occurrence of the day number identified by
d. n is a value between 1 and 4, or 5 meaning the last occurrence of
that weekday in the month (which could be the fourth or the fifth). d
is a value between 0 and 6, with 0 indicating Sunday.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

Robert Haas <robertmhaas@gmail.com> writes:
> You could consider something along the lines of:

> This form specifies a transition that always happens during the same
> month and on the same day of the week. m identifies the month, from 1
> to 12. n specifies the n'th occurrence of the day number identified by
> d. n is a value between 1 and 4, or 5 meaning the last occurrence of
> that weekday in the month (which could be the fourth or the fifth). d
> is a value between 0 and 6, with 0 indicating Sunday.

Adopted with some minor tweaks.  Thanks for the suggestion!

            regards, tom lane

I wrote:
> ... We should expect that, starting probably this fall, there
> will be installations with no posixrules file.

> The minimum thing that we have to do, I'd say, is to change the
> documentation to explain what happens if there's no posixrules file.
> However, in view of the fact that the posixrules feature doesn't work
> past 2037 and isn't going to be fixed, maybe we should just nuke it
> now rather than waiting for our hand to be forced.  I'm not sure that
> I've ever heard of anyone replacing the posixrules file anyway.
> (The fallback case is actually better in that it works for dates past
> 2037; it's worse only in that you can't configure it.)

I experimented with removing the posixrules support, and was quite glad
I did, because guess what: our regression tests fall over.  If we do
nothing we can expect that they'll start failing on various random systems
come this fall.

The cause of the failure is that we set the timezone for all regression
tests to just 'PST8PDT', which is exactly the underspecified POSIX syntax
that is affected by the posixrules feature.  So, with the fallback
rule of "M3.2.0,M11.1.0" (which corresponds to US law since 2007)
we get the wrong answers for some old test cases involving dates in 2005.

I'm inclined to think that the simplest fix is to replace 'PST8PDT' with
'America/Los_Angeles' as the standard zone setting for the regression
tests.  We definitely should be testing behavior with time-varying DST
laws, and we can no longer count on POSIX-style zone names to do that.

Another point, which I've not looked into yet, is that I'd always
supposed that PST8PDT and the other legacy US zone names would result
in loading the zone files of those names, ie /usr/share/zoneinfo/PST8PDT
and friends.  This seems not to be happening though.  Should we try
to make it happen?  It would probably result in fewer surprises once
posixrules goes away, because our regression tests are likely not the
only users of these zone names.

(I'd still be inclined to do the first thing though; it seems to me
that the historical behavior of 'America/Los_Angeles' is way more
likely to hold still than that of 'PST8PDT'.  The IANA crew might
nuke the latter zone entirely at some point, especially if the
repeated proposals to get rid of DST in the US ever get anywhere.)

            regards, tom lane

On 2020-06-17 20:08, Tom Lane wrote:
> I would definitely be in favor of "nuke it now" with respect to HEAD.
> It's a bit more debatable for the back branches.  However, all branches
> are going to be equally exposed to updated system tzdata trees, so
> we've typically felt that changes in the tz-related code should be
> back-patched.

It seems sensible to me to remove it in master and possibly 
REL_13_STABLE, but leave it alone in the back branches.

-- 
Peter Eisentraut              http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

I wrote:
> I experimented with removing the posixrules support, and was quite glad
> I did, because guess what: our regression tests fall over.  If we do
> nothing we can expect that they'll start failing on various random systems
> come this fall.

To clarify, you can produce this failure without any code changes:
build a standard installation (*not* using --with-system-tzdata),
remove its .../share/timezone/posixrules file, and run "make
installcheck".  So builds that do use --with-system-tzdata will fail both
"make check" and "make installcheck" if the platform's tzdata packager
decides to get rid of the posixrules file.

However, on closer inspection, all the test cases that depend on 'PST8PDT'
are fine, because we *do* pick up the zone file by that name.  The cases
that fall over are a few in horology.sql that depend on

SET TIME ZONE 'CST7CDT';

There is no such zone file, because that's a mistake: the central US
zone is more properly rendered 'CST6CDT'.  So this is indeed a bare
POSIX zone specification, and its behavior changes if there's no
posixrules file to back-fill knowledge about pre-2007 DST laws.

These test cases originated in commit b2b6548c7.  That was too long ago
to be sure, but I suspect that the use of a bogus zone was just a thinko;
there's certainly nothing in the commit log or the text of the patch
suggesting that it was intentional.  Still, it seems good to be testing
our POSIX-zone-string code paths, so I'm inclined to leave it as CST7CDT
but remove the dependence on posixrules by adding an explicit transition
rule.

Also, I notice a couple of related documentation issues:

* The same commit added a documentation example that also cites CST7CDT.
That needs to be fixed to correspond to something that would actually
be used in the real world, probably America/Denver.  Otherwise the
example will fail to work for some people.

* We should add something to the new appendix about POSIX zone specs
pointing out that while EST5EDT, CST6CDT, MST7MDT, PST8PDT look like they
are POSIX strings, they actually are captured by IANA zone files, so
that they produce valid historical US DST transitions even when a
plain POSIX string wouldn't.

I'm less excited than I was yesterday about removing the tests' dependency
on 'PST8PDT'.  It remains possible that we might need to do that someday,
but I doubt it'd happen without plenty of warning.

            regards, tom lane

Peter Eisentraut <peter.eisentraut@2ndquadrant.com> writes:
> On 2020-06-17 20:08, Tom Lane wrote:
>> I would definitely be in favor of "nuke it now" with respect to HEAD.
>> It's a bit more debatable for the back branches.  However, all branches
>> are going to be equally exposed to updated system tzdata trees, so
>> we've typically felt that changes in the tz-related code should be
>> back-patched.

> It seems sensible to me to remove it in master and possibly 
> REL_13_STABLE, but leave it alone in the back branches.

For purposes of discussion, here's a patch that rips out posixrules
support altogether.  (Note that further code simplifications could
be made --- the "load_ok" variable is vestigial, for instance.  This
formulation is intended to minimize the diffs from upstream.)

A less aggressive idea would be to leave the code alone and just change
the makefiles to not install a posixrules file in our own builds.
That'd leave the door open for somebody who really needed posixrules
behavior to get it back by just creating a posixrules file.  I'm not
sure this idea has much else to recommend it though.

I'm honestly not sure what I think we should do exactly.
The main arguments in favor of the full-rip-out option seem to be

(1) It'd ensure consistent behavior of POSIX zone specs across
platforms, whether or not --with-system-tzdata is used and whether
or not the platform supplies a posixrules file.

(2) We'll presumably be forced into the no-posixrules behavior at
some point, so forcing the issue lets us dictate the timing rather
than having it be dictated to us.  If nothing else, that means we
can release-note the behavioral change in a timely fashion.

Point (2) seems like an argument for doing it only in master
(possibly plus v13), but on the other hand I'm not convinced about
how much control we really have if we wait.  What seems likely
to happen is that posixrules files will disappear from platform
tz databases over some hard-to-predict timespan.  Even if no
major platforms drop them immediately at the next IANA update,
it seems quite likely that some/many will do so within the remaining
support lifetime of v12.  So even if we continue to support the feature,
it's likely to vanish in practice at some uncertain point.

Given that the issue only affects people using nonstandard TimeZone
settings, it may be that we shouldn't agonize over it too much
either way.

Anyway, as I write this I'm kind of talking myself into the position
that we should indeed back-patch this.  The apparent stability
benefits of not doing so may be illusory, and if we back-patch then
at least we get to document that there's a change.  But an argument
could be made in the other direction too.

Thoughts?

            regards, tom lane

diff --git a/doc/src/sgml/datetime.sgml b/doc/src/sgml/datetime.sgml
index 71fbf842cc..bbf50b76f8 100644
--- a/doc/src/sgml/datetime.sgml
+++ b/doc/src/sgml/datetime.sgml
@@ -718,33 +718,12 @@
   <para>
    If a daylight-savings abbreviation is given but the
    transition <replaceable>rule</replaceable> field is omitted,
-   <productname>PostgreSQL</productname> attempts to determine the
-   transition times by consulting the <filename>posixrules</filename> file
-   in the IANA time zone database.  This file has the same format as a
-   full time zone entry, but only its transition timing rules are used,
-   not its UTC offsets.  Typically, this file has the same contents as the
-   <literal>US/Eastern</literal> file, so that POSIX-style time zone
-   specifications follow USA daylight-savings rules.  If needed, you can
-   adjust this behavior by replacing the <filename>posixrules</filename>
-   file.
-  </para>
-
-  <note>
-   <para>
-    The facility to consult a <filename>posixrules</filename> file has
-    been deprecated by IANA, and it is likely to go away in the future.
-    One bug in this feature, which is unlikely to be fixed before it
-    disappears, is that it fails to apply DST rules to dates after 2038.
-   </para>
-  </note>
-
-  <para>
-   If the <filename>posixrules</filename> file is not present,
    the fallback behavior is to use the
    rule <literal>M3.2.0,M11.1.0</literal>, which corresponds to USA
    practice as of 2020 (that is, spring forward on the second Sunday of
    March, fall back on the first Sunday of November, both transitions
-   occurring at 2AM prevailing time).
+   occurring at 2AM prevailing time).  Note that this rule does not
+   give correct USA transition dates for years before 2007.
   </para>

   <para>
@@ -765,8 +744,7 @@
    because (for historical reasons) there are files by those names in the
    IANA time zone database.  The practical implication of this is that
    these zone names will produce valid historical USA daylight-savings
-   transitions, even when a plain POSIX specification would not due to
-   lack of a suitable <filename>posixrules</filename> file.
+   transitions, even when a plain POSIX specification would not.
   </para>

   <para>
diff --git a/src/timezone/Makefile b/src/timezone/Makefile
index bf23ac9da9..715b63cee0 100644
--- a/src/timezone/Makefile
+++ b/src/timezone/Makefile
@@ -29,10 +29,6 @@ ZICOBJS = \
 # we now distribute the timezone data as a single file
 TZDATAFILES = $(srcdir)/data/tzdata.zi

-# which zone should determine the DST rules (not the specific UTC offset!)
-# for POSIX-style timezone specs
-POSIXRULES = US/Eastern
-
 # any custom options you might want to pass to zic while installing data files
 ZIC_OPTIONS =

@@ -60,13 +56,13 @@ zic: $(ZICOBJS) | submake-libpgport

 install: all installdirs
 ifeq (,$(with_system_tzdata))
-    $(ZIC) -d '$(DESTDIR)$(datadir)/timezone' -p '$(POSIXRULES)' -b slim $(ZIC_OPTIONS) $(TZDATAFILES)
+    $(ZIC) -d '$(DESTDIR)$(datadir)/timezone' -b slim $(ZIC_OPTIONS) $(TZDATAFILES)
 endif
     $(MAKE) -C tznames $@

 abbrevs.txt: zic $(TZDATAFILES)
     mkdir junkdir
-    $(ZIC) -P -d junkdir -p '$(POSIXRULES)' $(TZDATAFILES) | LANG=C sort | uniq >abbrevs.txt
+    $(ZIC) -P -d junkdir $(TZDATAFILES) | LANG=C sort | uniq >abbrevs.txt
     rm -rf junkdir

 installdirs:
diff --git a/src/timezone/README b/src/timezone/README
index 9939aa6dd7..8af4444932 100644
--- a/src/timezone/README
+++ b/src/timezone/README
@@ -93,10 +93,7 @@ in some other files where we have variables named that.
 slightly modified the API of the former, in part because it now relies
 on our own pg_open_tzfile() rather than opening files for itself.

-* tzparse() is adjusted to avoid loading the TZDEFRULES zone unless
-really necessary, and to ignore any leap-second data it may supply.
-We also cache the result of loading the TZDEFRULES zone, so that
-that's not repeated more than once per process.
+* tzparse() is adjusted to never try to load the TZDEFRULES zone.

 * There's a fair amount of code we don't need and have removed,
 including all the nonstandard optional APIs.  We have also added
diff --git a/src/timezone/localtime.c b/src/timezone/localtime.c
index 0f65f3c648..fa3c059038 100644
--- a/src/timezone/localtime.c
+++ b/src/timezone/localtime.c
@@ -53,14 +53,7 @@ static const char wildabbr[] = WILDABBR;
 static const char gmt[] = "GMT";

 /*
- * PG: We cache the result of trying to load the TZDEFRULES zone here.
- * tzdefrules_loaded is 0 if not tried yet, +1 if good, -1 if failed.
- */
-static struct state *tzdefrules_s = NULL;
-static int    tzdefrules_loaded = 0;
-
-/*
- * The DST rules to use if TZ has no rules and we can't load TZDEFRULES.
+ * The DST rules to use if a POSIX TZ string has no rules.
  * Default to US rules as of 2017-05-07.
  * POSIX does not specify the default DST rules;
  * for historical reasons, US rules are a common default.
@@ -986,14 +979,15 @@ tzparse(const char *name, struct state *sp, bool lastditch)
         return false;

     /*
-     * The IANA code always tries tzload(TZDEFRULES) here.  We do not want to
-     * do that; it would be bad news in the lastditch case, where we can't
-     * assume pg_open_tzfile() is sane yet.  Moreover, the only reason to do
-     * it unconditionally is to absorb the TZDEFRULES zone's leap second info,
-     * which we don't want to do anyway.  Without that, we only need to load
-     * TZDEFRULES if the zone name specifies DST but doesn't incorporate a
-     * POSIX-style transition date rule, which is not a common case.
+     * The IANA code always tries to tzload(TZDEFRULES) here.  We do not want
+     * to do that; it would be bad news in the lastditch case, where we can't
+     * assume pg_open_tzfile() is sane yet.  Moreover, if we did load it and
+     * it contains leap-second-dependent info, that would cause problems too.
+     * Finally, IANA has deprecated the TZDEFRULES feature, so it presumably
+     * will die at some point.  Desupporting it now seems like good
+     * future-proofing.
      */
+    load_ok = false;
     sp->goback = sp->goahead = false;    /* simulate failed tzload() */
     sp->leapcnt = 0;            /* intentionally assume no leap seconds */

@@ -1027,38 +1021,8 @@ tzparse(const char *name, struct state *sp, bool lastditch)
         }
         else
             dstoffset = stdoffset - SECSPERHOUR;
-        if (*name == '\0')
-        {
-            /*
-             * The POSIX zone name does not provide a transition-date rule.
-             * Here we must load the TZDEFRULES zone, if possible, to serve as
-             * source data for the transition dates.  Unlike the IANA code, we
-             * try to cache the data so it's only loaded once.
-             */
-            if (tzdefrules_loaded == 0)
-            {
-                /* Allocate on first use */
-                if (tzdefrules_s == NULL)
-                    tzdefrules_s = (struct state *) malloc(sizeof(struct state));
-                if (tzdefrules_s != NULL)
-                {
-                    if (tzload(TZDEFRULES, NULL, tzdefrules_s, false) == 0)
-                        tzdefrules_loaded = 1;
-                    else
-                        tzdefrules_loaded = -1;
-                    /* In any case, we ignore leap-second data from the file */
-                    tzdefrules_s->leapcnt = 0;
-                }
-            }
-            load_ok = (tzdefrules_loaded > 0);
-            if (load_ok)
-                memcpy(sp, tzdefrules_s, sizeof(struct state));
-            else
-            {
-                /* If we can't load TZDEFRULES, fall back to hard-wired rule */
-                name = TZDEFRULESTRING;
-            }
-        }
+        if (*name == '\0' && !load_ok)
+            name = TZDEFRULESTRING;
         if (*name == ',' || *name == ';')
         {
             struct rule start;
diff --git a/src/tools/msvc/Install.pm b/src/tools/msvc/Install.pm
index 9bf111c41e..b6d0cfd39b 100644
--- a/src/tools/msvc/Install.pm
+++ b/src/tools/msvc/Install.pm
@@ -366,16 +366,10 @@ sub GenerateTimezoneFiles
       || die "Could not find TZDATAFILES line in timezone makefile\n";
     my @tzfiles = split /\s+/, $1;

-    $mf =~ /^POSIXRULES\s*:?=\s*(.*)$/m
-      || die "Could not find POSIXRULES line in timezone makefile\n";
-    my $posixrules = $1;
-    $posixrules =~ s/\s+//g;
-
     print "Generating timezone files...";

     my @args = (
-        "$conf/zic/zic", '-d', "$target/share/timezone", '-p',
-        "$posixrules",   '-b', 'slim');
+        "$conf/zic/zic", '-d', "$target/share/timezone", '-b', 'slim');
     foreach (@tzfiles)
     {
         my $tzfile = $_;

On Fri, Jun 19, 2020 at 3:27 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
> Anyway, as I write this I'm kind of talking myself into the position
> that we should indeed back-patch this.  The apparent stability
> benefits of not doing so may be illusory, and if we back-patch then
> at least we get to document that there's a change.  But an argument
> could be made in the other direction too.

It's really unclear to me why we should back-patch this into
already-released branches. I grant your point that perhaps few people
will notice, and also that this might happen at some point the change
will be forced upon us. Nonetheless, we bill our back-branches as
being stable, which seems inconsistent with forcing a potentially
breaking change into them without a clear and pressing need. If you
commit this patch to master and v13, no already-release branches will
be affected immediately, and it's conceivable that some or even all of
the older branches will age out before the issue is forced. That would
be all to the good. And even if the issue is forced sooner rather than
later, how much do we really lose by waiting until we have that
problem in front of us?

I'm not in a position to judge how much additional maintenance
overhead would be imposed by not back-patching this at once, so if you
tell me that it's an intolerable burden, I can't really argue with
that. But if it's possible to take a wait-and-see attitude for the
time being, so much the better.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

Robert Haas <robertmhaas@gmail.com> writes:
> It's really unclear to me why we should back-patch this into
> already-released branches. I grant your point that perhaps few people
> will notice, and also that this might happen at some point the change
> will be forced upon us. Nonetheless, we bill our back-branches as
> being stable, which seems inconsistent with forcing a potentially
> breaking change into them without a clear and pressing need. If you
> commit this patch to master and v13, no already-release branches will
> be affected immediately, and it's conceivable that some or even all of
> the older branches will age out before the issue is forced. That would
> be all to the good. And even if the issue is forced sooner rather than
> later, how much do we really lose by waiting until we have that
> problem in front of us?

> I'm not in a position to judge how much additional maintenance
> overhead would be imposed by not back-patching this at once, so if you
> tell me that it's an intolerable burden, I can't really argue with
> that. But if it's possible to take a wait-and-see attitude for the
> time being, so much the better.

The code delta is small enough that I don't foresee any real maintenance
problem if we let the back branches differ from HEAD/v13 on this point.
What I'm concerned about is that people depending on the existing
behavior are likely to wake up one fine morning and discover that it's
broken after a routine tzdata update.  I think that it'd be a better
user experience for them to see a release-note entry in a PG update
release explaining that this will break and here's what to do to fix it.

Yeah, we can do nothing in the back branches and hope that that doesn't
happen for the remaining lifespan of v12.  But I wonder whether that
doesn't amount to sticking our heads in the sand.

I suppose it'd be possible to have a release-note entry in the back
branches that isn't tied to any actual code change on our part, but just
warns that such a tzdata change might happen at some unpredictable future
time.  That feels weird and squishy though; and people would likely have
forgotten it by the time the change actually hits them.

            regards, tom lane

On Fri, Jun 19, 2020 at 3:55 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
> The code delta is small enough that I don't foresee any real maintenance
> problem if we let the back branches differ from HEAD/v13 on this point.
> What I'm concerned about is that people depending on the existing
> behavior are likely to wake up one fine morning and discover that it's
> broken after a routine tzdata update.  I think that it'd be a better
> user experience for them to see a release-note entry in a PG update
> release explaining that this will break and here's what to do to fix it.

I was assuming that if you did an update of the tzdata, you'd notice
if posixrules had been nuked. I guess that wouldn't help people who
are using the system tzdata, though. It might be nice to know what
Debian, RHEL, etc. plan to do about this, but I'm not sure how
practical it is to find out.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

Robert Haas <robertmhaas@gmail.com> writes:
> On Fri, Jun 19, 2020 at 3:55 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> What I'm concerned about is that people depending on the existing
>> behavior are likely to wake up one fine morning and discover that it's
>> broken after a routine tzdata update.  I think that it'd be a better
>> user experience for them to see a release-note entry in a PG update
>> release explaining that this will break and here's what to do to fix it.

> I was assuming that if you did an update of the tzdata, you'd notice
> if posixrules had been nuked. I guess that wouldn't help people who
> are using the system tzdata, though.

Yeah, exactly.  We can control this easily enough for PG-supplied tzdata
trees, but I think a significant majority of actual users are using
--with-system-tzdata builds, because we've been telling packagers to
do it that way for years.  (Nor does changing that advice seem like
a smart move.)

> It might be nice to know what
> Debian, RHEL, etc. plan to do about this, but I'm not sure how
> practical it is to find out.

There's probably no way to know until it happens :-(.  We can hope
that they'll be conservative, but it's hard to be sure.  It doesn't
help that the bigger players rely on glibc: if I understand what
Eggert was saying, nuking posixrules would bring tzcode's behavior
into closer sync with what glibc does, so they might well feel it's
a desirable change.

            regards, tom lane

I wrote:
> Robert Haas <robertmhaas@gmail.com> writes:
>> It might be nice to know what
>> Debian, RHEL, etc. plan to do about this, but I'm not sure how
>> practical it is to find out.

> There's probably no way to know until it happens :-(.

On the other hand, for the open-source players, it might be easier to
guess.  I took a look at the Fedora/RHEL tzdata specfile, and I see
that "-p America/New_York" is hard-wired into it:

zic -y ./yearistype -d zoneinfo -L /dev/null -p America/New_York $FILES

This means that IANA's change of their sample Makefile will have no
direct impact, and things will only change if the Red Hat packager
actively changes the specfile.  It's still anyone's guess whether
he/she will do so, but the odds of a change seem a good bit lower
than if the IANA-supplied Makefile were being used directly.

I'm less familiar with Debian so I won't venture to dig into their
package, but maybe somebody else would like to.

            regards, tom lane

Robert Haas <robertmhaas@gmail.com> writes:
> It might be nice to know what
> Debian, RHEL, etc. plan to do about this, but I'm not sure how
> practical it is to find out.

By luck, we now have a moderately well-educated guess about that
from Paul Eggert himself [1]:

: Probably NetBSD will go first as they tend to buy these changes
: quickly; maybe six months from now? Debian and RHEL probably a couple
: of years. These are all just guesses.

Based on that, I'd say that assuming v12 and earlier won't have to deal
with this issue does indeed amount to sticking our heads in the sand.

I don't intend to do anything about this until this week's beta wrap
cycle is complete, but I'm still leaning to the idea that we ought to
back-patch something.  Maybe the "something" could be less than a
full posixrules-ectomy, but I'm not really satisfied with any of the
other alternatives I've thought about.

            regards, tom lane

[1] https://www.postgresql.org/message-id/9d8b5ec4-7094-04f6-d270-db0198d09bd1%40cs.ucla.edu

On 2020-06-19 21:55, Tom Lane wrote:
> Yeah, we can do nothing in the back branches and hope that that doesn't
> happen for the remaining lifespan of v12.  But I wonder whether that
> doesn't amount to sticking our heads in the sand.
> 
> I suppose it'd be possible to have a release-note entry in the back
> branches that isn't tied to any actual code change on our part, but just
> warns that such a tzdata change might happen at some unpredictable future
> time.  That feels weird and squishy though; and people would likely have
> forgotten it by the time the change actually hits them.

In my mind, this isn't really that different from other external 
libraries making API changes.  But we are not going to forcibly remove 
Python 2 support in PostgreSQL 9.6 just because it's no longer supported 
upstream.  If Debian or RHEL $veryold want to keep maintaining Python 2, 
they are free to do so, and users thereof are free to continue using it. 
  Similarly, Debian or RHEL $veryold are surely not going to drop a 
whole class of time zone codes from their stable distribution just 
because upstream is phasing it out.

What you are saying is, instead of the OS dropping POSIXRULES support, 
it would be better if we dropped it first and release-noted that. 
However, I don't agree with the premise of that.  OSes with long-term 
support aren't going to drop it.

-- 
Peter Eisentraut              http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

Peter Eisentraut <peter.eisentraut@2ndquadrant.com> writes:
> What you are saying is, instead of the OS dropping POSIXRULES support, 
> it would be better if we dropped it first and release-noted that. 
> However, I don't agree with the premise of that.  OSes with long-term 
> support aren't going to drop it.

You might be right, or you might not.  I think the tzdata distribution is
in a weird gray area so far as long-term-support platforms are concerned:
they have to keep updating it, no matter how far it diverges from what
they originally shipped with.  Maybe they will figure out that they're not
required to drop POSIXRULES just because upstream did.  Or maybe they will
go with the flow on that, figuring that it's not any worse than any
politically-driven time zone change.

I wouldn't be a bit surprised if it ends up depending on whether the
particular distro is using IANA's makefile more or less verbatim.
In Red Hat's case I found that they'd have to take positive action to
drop POSIXRULES, so I'd agree that it won't happen there for a long time,
and not in any existing RHEL release.  In some other distros, it might
take explicit addition of a patch to keep from dropping POSIXRULES, in
which case I think there'd be quite good odds that that won't happen
and the changeover occurs with the next IANA zone updates.

The nasty thing about that scenario from our perspective is that it
means that the same timezone spec means different things on different
platforms, even ones nominally using the same tzdata release.  Do we
want to deal with that, or take pre-emptive action to prevent it?

(You could argue that that hazard already exists for people who are
intentionally using nonstandard posixrules files.  But I think the
set of such people can be counted without running out of fingers.
If there's some evidence to the contrary I'd like to see it.)

I'm also worried about what the endgame looks like.  It seems clear
that at some point IANA is going to remove their code's support for
reading a posixrules file.  Eggert hasn't tipped his hand as to when
he thinks that might happen, but I wouldn't care to bet that it's
more than five years away.  I don't want to find ourselves in a
situation where we have to maintain code that upstream has nuked.
If they only do something comparable to the patch I posted, it
wouldn't be so bad; but if they then undertake any significant
follow-on cleanup we'd be in a very bad place for tracking them.

            regards, tom lane

Seems like I'm not getting any traction in convincing people that
back-patching this change is wise.  To get this closed out before
the CF starts, I'm just going to put it into HEAD/v13 and call it
a day.

I remain of the opinion that we'll probably regret not doing
anything in the back branches, sometime in the next 4+ years.

            regards, tom lane