Re: pg_upgrade fails with non-standard ACL

HelLo!

On 11/9/19 5:26 AM, Michael Paquier wrote:
> On Fri, Nov 08, 2019 at 06:03:06PM +0900, Michael Paquier wrote:
>> I have begun looking at this one.
> Another question I have: do we need to care more about other extra
> ACLs applied to other object types? For example a subset of columns
> on a table with a column being renamed could be an issue. Procedure
> renamed in core are not that common still we did it.

I think that all objects must be supported.
User application may depend on them, so silently casting them to the
void during upgrade may ruin someones day.
But also I think, that this work should be done piecemeal, to make
testing and reviewing easier, and functions are a good candidate for a
first go.

> I think that we should keep the core part of the fix more simple by
> just warning about missing function signatures in the target cluster
> when pg_upgrade --check is used.

I think that warning without any concrete details on functions involved
is confusing.
> So I think that it would be better
> for now to get to a point where we can warn about the function
> signatures involved in a given database, without the generation of the
> script with those REVOKE queries. Or would folks prefer keep that in
> the first version? My take would be to handle both separately, and to
> warn about everything so as there is no need to do pg_upgrade --check
> more than once.

I would prefer to warn about every function (so he can more quickly
assess the situation)  AND generate script. It is good to save some user
time, because he is going to need that script anyway.

I`ve tested the master patch:

1. upgrade from 9.5 and lower is broken:

[gsmol(at)deck upgrade_workdir]$ /home/gsmol/task/13_devel/bin/pg_upgrade
-b /home/gsmol/task/9.5.19/bin/ -B /home/gsmol/task/13_devel/bin/ -d
/home/gsmol/task/9.5.19/data -D /tmp/upgrade
Performing Consistency Checks
-----------------------------
Checking cluster versions                                   ok
SQL command failed
select proname::text || '(' ||
pg_get_function_arguments(pg_proc.oid)::text || ')' as funsig, array
(SELECT unnest(pg_proc.proacl) EXCEPT SELECT
unnest(pg_init_privs.initprivs)) from pg_proc join pg_init_privs on
pg_proc.oid = pg_init_privs.objoid where pg_init_privs.classoid =
'pg_proc'::regclass::oid and pg_init_privs.privtype = 'i' and
pg_proc.proisagg = false and pg_proc.proacl != pg_init_privs.initprivs;
ERROR:  relation "pg_init_privs" does not exist
LINE 1: ...nnest(pg_init_privs.initprivs)) from pg_proc join pg_init_pr...
                                                             ^
Failure, exiting

2. I think that message "Your installation contains non-default
privileges for system procedures for which the API has changed." must
contain versions:
"Your PostgreSQL 9.5 installation contains non-default privileges for
system procedures for which the API has changed in PostgreSQL 12."

--
Grigory Smolkin
Postgres Professional: http://www.postgrespro.com
The Russian Postgres Company