Re: pg_upgrade fails with non-standard ACL - Mailing list pgsql-hackers

From Grigory Smolkin
Subject Re: pg_upgrade fails with non-standard ACL
Date
Msg-id 03c2a02d-befa-f2e7-f698-e85f4ee99674@postgrespro.ru
Whole thread Raw
In response to Re: pg_upgrade fails with non-standard ACL  (Michael Paquier <michael@paquier.xyz>)
Responses Re: pg_upgrade fails with non-standard ACL
List pgsql-hackers
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@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




pgsql-hackers by date:

Previous
From: Pavel Stehule
Date:
Subject: Re: dropdb --force
Next
From: Pantelis Theodosiou
Date:
Subject: Re: [PATCH] Implement INSERT SET syntax