From: Tom Lane <tgl@sss.pgh.pa.us>
To: Daniel Gustafsson <daniel@yesql.se>
cc: Alvaro Herrera <alvherre@2ndquadrant.com>,
        Postgres hackers <pgsql-hackers@lists.postgresql.org>
Subject: Re: Retiring support for pre-7.3 FK constraint triggers
In-reply-to: <B3B93FAD-5B8C-4594-A33B-CAE504520530@yesql.se>
References: <20200305143041.GA7031@alvherre.pgsql>
 <1154.1583419325@sss.pgh.pa.us>
 <B3B93FAD-5B8C-4594-A33B-CAE504520530@yesql.se>
Comments: In-reply-to Daniel Gustafsson <daniel@yesql.se>
	message dated "Thu, 05 Mar 2020 16:12:14 +0100"
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-ID: <3684.1583422389.1@sss.pgh.pa.us>
Content-Transfer-Encoding: quoted-printable
Date: Thu, 05 Mar 2020 10:33:09 -0500
Message-ID: <3685.1583422389@sss.pgh.pa.us>
Precedence: bulk

Daniel Gustafsson <daniel@yesql.se> writes:
> On 5 Mar 2020, at 15:42, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> +1 --- I think this fits in well with my nearby proposal to remove
>> OPAQUE, which is also only relevant for pre-7.3 dumps.  Let's just
>> nuke that stuff.

> Sounds good. I was opting for 14 to not violate the no new patches in an=
 ongoing CF policy, but if there is concensus from committers then +1 from=
 me.

As long as we're thinking of zapping code that is long past its sell-by
date, I propose getting rid of this stanza in indexcmds.c, which
basically causes CREATE INDEX to ignore certain opclass specifications:

    /*
     * Release 7.0 removed network_ops, timespan_ops, and datetime_ops, so=
 we
     * ignore those opclass names so the default *_ops is used.  This can =
be
     * removed in some later release.  bjm 2000/02/07
     *
     * Release 7.1 removes lztext_ops, so suppress that too for a while.  =
tgl
     * 2000/07/30
     *
     * Release 7.2 renames timestamp_ops to timestamptz_ops, so suppress t=
hat
     * too for awhile.  I'm starting to think we need a better approach. t=
gl
     * 2000/10/01
     *
     * Release 8.0 removes bigbox_ops (which was dead code for a long whil=
e
     * anyway).  tgl 2003/11/11
     */
    if (list_length(opclass) =3D=3D 1)
    {
        char       *claname =3D strVal(linitial(opclass));

        if (strcmp(claname, "network_ops") =3D=3D 0 ||
            strcmp(claname, "timespan_ops") =3D=3D 0 ||
            strcmp(claname, "datetime_ops") =3D=3D 0 ||
            strcmp(claname, "lztext_ops") =3D=3D 0 ||
            strcmp(claname, "timestamp_ops") =3D=3D 0 ||
            strcmp(claname, "bigbox_ops") =3D=3D 0)
            opclass =3D NIL;
    }


At some point, the risk that this causes problems for developers of
new opclasses must outweigh the value of silently upgrading old dumps.
I think if we're zapping other pre-7.3-compatibility hacks for that
purpose, this one could go too.

Elsewhere in indexcmds.c, there's this:

        /*
         * Hack to provide more-or-less-transparent updating of old RTREE
         * indexes to GiST: if RTREE is requested and not found, use GIST.
         */
        if (strcmp(accessMethodName, "rtree") =3D=3D 0)
        {
            ereport(NOTICE,
                    (errmsg("substituting access method \"gist\" for obsol=
ete method \"rtree\"")));
            accessMethodName =3D "gist";
            tuple =3D SearchSysCache1(AMNAME, PointerGetDatum(accessMethod=
Name));
        }

which dates to 8.2 (2a8d3d83e of 2005-11-07).  This is less bad than the
other thing, since it won't affect the behavior of any command that
wouldn't otherwise just fail; but maybe its time has passed as well?
Although Alvaro's point comparing these behaviors to pg_dump's support
cutoff of 8.0 suggests that maybe we should leave this one for now.

			regards, tom lane