From: Tom Lane <tgl@sss.pgh.pa.us>
To: Justin Pryzby <pryzby@telsasoft.com>
cc: "David G. Johnston" <david.g.johnston@gmail.com>,
        Petr Vejsada <pve@paymorrow.com>, pgsql-hackers@lists.postgresql.org
Subject: Re: pg_upgrade (12->14) fails on aggregate
In-reply-to: <20220614230949.GX29853@telsasoft.com>
References: <3383880.QJadu78ljV@vejsadalnx>
 <CAKFQuwbUUwkO0ffRY4+z4Hs9-jRjAUwGHZrDKyHhRudWVhtgkw@mail.gmail.com>
 <20220614230949.GX29853@telsasoft.com>
Comments: In-reply-to Justin Pryzby <pryzby@telsasoft.com>
	message dated "Tue, 14 Jun 2022 18:09:49 -0500"
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-ID: <540146.1655321524.1@sss.pgh.pa.us>
Date: Wed, 15 Jun 2022 15:32:04 -0400
Message-ID: <540147.1655321524@sss.pgh.pa.us>
Archived-At: 
 <https://www.postgresql.org/message-id/540147.1655321524%40sss.pgh.pa.us>
Precedence: bulk

Justin Pryzby <pryzby@telsasoft.com> writes:
> But Petr has a point - pg_upgrade should aspire to catch errors in --check,
> rather than starting and then leaving a mess behind for the user to clean up

Agreed; pg_upgrade has historically tried to find problems similar to
this.  However, it's not just aggregates that are at risk.  People
might also have built user-defined plain functions, or operators,
atop these functions.  How far do we want to go in looking?

As for the query, I think it could be simplified quite a bit by
relying on regprocedure literals, that is something like

WHERE ... a.aggtransfn IN
      ('array_append(anyarray,anyelement)'::regprocedure,
       'array_prepend(anyelement,anyarray)'::regprocedure,
       ...)

Not sure if it's necessary to stick explicit "pg_catalog." schema
qualifications into this --- IIRC pg_upgrade runs with restrictive
search_path, so that this would be safe as-is.

Also, I think you need to check aggfinalfn too.

Also, I'd be inclined to reject system-provided objects by checking
for OID >= 16384 rather than hard-wiring assumptions about things
being in pg_catalog or not.

			regards, tom lane