Re: pg_upgrade fails with non-standard ACL

2021-11-26 Thread Daniel Gustafsson
This patch has been marked Waiting on Author since early March, with the thread stalled since then. I'm marking this CF entry Returned with Feedback, please feel free to resubmit it if/when a new version of the patch is available. -- Daniel Gustafsson https://vmware.com/

Re: pg_upgrade fails with non-standard ACL

2021-03-08 Thread Noah Misch
On Thu, Feb 11, 2021 at 08:16:30PM +0300, Anastasia Lubennikova wrote: > On 28.01.2021 09:55, Noah Misch wrote: > >On Wed, Jan 27, 2021 at 07:32:42PM +0300, Anastasia Lubennikova wrote: > >>On 27.01.2021 14:21, Noah Misch wrote: > >>>On Mon, Jan 25, 2021 at 10:14:43PM +0300, Anastasia Lubennikova

Re: pg_upgrade fails with non-standard ACL

2021-01-27 Thread Noah Misch
On Wed, Jan 27, 2021 at 07:32:42PM +0300, Anastasia Lubennikova wrote: > On 27.01.2021 14:21, Noah Misch wrote: > >On Mon, Jan 25, 2021 at 10:14:43PM +0300, Anastasia Lubennikova wrote: > > > >>1) Could you please clarify, what do you mean by REVOKE failures? > >> > >>I tried following example: >

Re: pg_upgrade fails with non-standard ACL

2021-01-27 Thread Noah Misch
On Mon, Jan 25, 2021 at 10:14:43PM +0300, Anastasia Lubennikova wrote: > On 24.01.2021 11:39, Noah Misch wrote: > >On Thu, Jan 21, 2021 at 01:03:58AM +0300, Anastasia Lubennikova wrote: > >>On 03.01.2021 14:29, Noah Misch wrote: > >>>Overall, this patch predicts a subset of cases where pg_dump

Re: pg_upgrade fails with non-standard ACL

2021-01-25 Thread Anastasia Lubennikova
On 24.01.2021 11:39, Noah Misch wrote: On Thu, Jan 21, 2021 at 01:03:58AM +0300, Anastasia Lubennikova wrote: On 03.01.2021 14:29, Noah Misch wrote: Overall, this patch predicts a subset of cases where pg_dump will emit a failing GRANT or REVOKE that targets a pg_catalog object. Can you write

Re: pg_upgrade fails with non-standard ACL

2021-01-24 Thread Noah Misch
On Thu, Jan 21, 2021 at 01:03:58AM +0300, Anastasia Lubennikova wrote: > On 03.01.2021 14:29, Noah Misch wrote: > >Overall, this patch predicts a subset of cases where pg_dump will emit a > >failing GRANT or REVOKE that targets a pg_catalog object. Can you write a > >code comment stating what it

Re: pg_upgrade fails with non-standard ACL

2021-01-20 Thread Noah Misch
On Thu, Jan 21, 2021 at 01:03:58AM +0300, Anastasia Lubennikova wrote: > On 03.01.2021 14:29, Noah Misch wrote: > >On Thu, Jun 11, 2020 at 07:58:43PM +0300, Anastasia Lubennikova wrote: > Thank you for the review. > New version of the patch is attached, though I haven't tested it properly > yet.

Re: pg_upgrade fails with non-standard ACL

2021-01-20 Thread Anastasia Lubennikova
On 03.01.2021 14:29, Noah Misch wrote: On Thu, Jun 11, 2020 at 07:58:43PM +0300, Anastasia Lubennikova wrote: On 08.06.2020 19:31, Alvaro Herrera wrote: I'm thinking what's a good way to have a test that's committable. Maybe it would work to add a TAP test to pg_upgrade that runs initdb, does

Re: pg_upgrade fails with non-standard ACL

2021-01-03 Thread Noah Misch
On Thu, Jun 11, 2020 at 07:58:43PM +0300, Anastasia Lubennikova wrote: > On 08.06.2020 19:31, Alvaro Herrera wrote: > >I'm thinking what's a good way to have a test that's committable. Maybe > >it would work to add a TAP test to pg_upgrade that runs initdb, does a > >few GRANTS as per your

Re: pg_upgrade fails with non-standard ACL

2020-11-23 Thread Grigory Smolkin
Tested this patch by running several upgrades from different major versions and different setups. ACL, that are impossible to apply, are detected and reported, so it looks good for me.

Re: pg_upgrade fails with non-standard ACL

2020-06-12 Thread Michael Paquier
On Thu, Jun 11, 2020 at 07:58:43PM +0300, Anastasia Lubennikova wrote: > I would be glad to add some test, but it seems to me that the infrastructure > changes for cross-version pg_upgrade test is much more complicated task than > this modest bugfix.  Besides, I've read the discussion and it seems

Re: pg_upgrade fails with non-standard ACL

2020-06-11 Thread Anastasia Lubennikova
On 08.06.2020 19:31, Alvaro Herrera wrote: On 2020-Jun-08, Anastasia Lubennikova wrote: In this version I rebased test patches,  added some more comments, fixed memory allocation issue and also removed code that handles ACLs on languages. They require a lot of specific handling, while I doubt

Re: pg_upgrade fails with non-standard ACL

2020-06-08 Thread Alvaro Herrera
On 2020-Jun-08, Anastasia Lubennikova wrote: > In this version I rebased test patches,  added some more comments, fixed > memory allocation issue and also removed code that handles ACLs on > languages. They require a lot of specific handling, while I doubt that their > signatures, which consist

Re: pg_upgrade fails with non-standard ACL

2020-06-08 Thread Anastasia Lubennikova
On 06.04.2020 19:40, Anastasia Lubennikova wrote: On 06.04.2020 16:49, Anastasia Lubennikova wrote: New version of the patch is attached. I fixed several issues in the check_for_changed_signatures(). Now it passes check without "test_rename_catalog_objects" and fails (generates script) with

Re: pg_upgrade fails with non-standard ACL

2020-04-06 Thread Anastasia Lubennikova
On 06.04.2020 16:49, Anastasia Lubennikova wrote: New version of the patch is attached. I fixed several issues in the check_for_changed_signatures(). Now it passes check without "test_rename_catalog_objects" and fails (generates script) with it. Test script pg_upgrade_ACL_test.sh

Re: pg_upgrade fails with non-standard ACL

2020-04-06 Thread Anastasia Lubennikova
New version of the patch is attached. I fixed several issues in the check_for_changed_signatures(). Now it passes check without "test_rename_catalog_objects" and fails (generates script) with it. Test script pg_upgrade_ACL_test.sh demonstrates this. The only known issue left is the usage of

Re: pg_upgrade fails with non-standard ACL

2020-03-25 Thread Justin Pryzby
On Wed, Mar 25, 2020 at 11:12:05AM +0900, Artur Zakirov wrote: > Hello David, > > On 3/25/2020 2:08 AM, David Steele wrote: > > On 12/17/19 3:10 AM, Arthur Zakirov wrote: > > > > > > I attached new version of the patch. It still uses > > > pg_identify_object(), I'm not sure about other ways to

Re: pg_upgrade fails with non-standard ACL

2020-03-25 Thread Daniel Gustafsson
> On 25 Mar 2020, at 03:12, Artur Zakirov wrote: > Regression tests fail because cfbot applies > "test_rename_catalog_objects.patch". Regression tests pass without it. > > This patch shouldn't be applied by cfbot. I'm not sure how to do this. But > maybe it is possible to use different

Re: pg_upgrade fails with non-standard ACL

2020-03-24 Thread Artur Zakirov
Hello David, On 3/25/2020 2:08 AM, David Steele wrote: On 12/17/19 3:10 AM, Arthur Zakirov wrote: I attached new version of the patch. It still uses pg_identify_object(), I'm not sure about other ways to build identities yet. This patch applies and builds but fails regression tests on

Re: pg_upgrade fails with non-standard ACL

2020-03-24 Thread David Steele
On 12/17/19 3:10 AM, Arthur Zakirov wrote: I attached new version of the patch. It still uses pg_identify_object(), I'm not sure about other ways to build identities yet. This patch applies and builds but fails regression tests on Linux and Windows:

Re: pg_upgrade fails with non-standard ACL

2020-03-24 Thread Anastasia Lubennikova
On 17.12.2019 11:10, Arthur Zakirov wrote: On 2019/12/05 11:31, Michael Paquier wrote: On Wed, Dec 04, 2019 at 06:15:52PM +0900, Arthur Zakirov wrote: Ah, I thought that pg_identify_object() gives properly quoted identity, and it could be used to make SQL script. It depends on the object

Re: pg_upgrade fails with non-standard ACL

2019-12-17 Thread Arthur Zakirov
Hello, On 2019/12/05 11:31, Michael Paquier wrote: On Wed, Dec 04, 2019 at 06:15:52PM +0900, Arthur Zakirov wrote: Ah, I thought that pg_identify_object() gives properly quoted identity, and it could be used to make SQL script. It depends on the object type. For columns I can see in your

Re: pg_upgrade fails with non-standard ACL

2019-12-04 Thread Michael Paquier
On Wed, Dec 04, 2019 at 06:15:52PM +0900, Arthur Zakirov wrote: > On 2019/12/04 17:15, Michael Paquier wrote: >> FWIW, I am not much a fan of that part because the output generated by >> the description is most likely not compatible with the grammar >> supported. > > Ah, I thought that

Re: pg_upgrade fails with non-standard ACL

2019-12-04 Thread Arthur Zakirov
On 2019/12/04 17:15, Michael Paquier wrote: On Wed, Dec 04, 2019 at 12:17:25PM +0900, Arthur Zakirov wrote: I updated the patch. It generates "revoke_objects.sql" (similar to v3 patch) now and doesn't rely on --check option. It also logs still FATAL message because it seems pg_upgrade should

Re: pg_upgrade fails with non-standard ACL

2019-12-04 Thread Michael Paquier
On Wed, Dec 04, 2019 at 12:17:25PM +0900, Arthur Zakirov wrote: > I updated the patch. It generates "revoke_objects.sql" (similar to v3 patch) > now and doesn't rely on --check option. It also logs still FATAL message > because it seems pg_upgrade should stop here since it fails later if there >

Re: pg_upgrade fails with non-standard ACL

2019-12-03 Thread Arthur Zakirov
On 2019/12/01 23:58, Grigory Smolkin wrote: On 11/29/19 11:07 AM, Artur Zakirov wrote: New version of the patch differs from the previous: - it doesn't generate script to revoke conflicting permissions (but the patch can be fixed easily) - generates file incompatible_objects_for_acl.txt to

Re: pg_upgrade fails with non-standard ACL

2019-12-01 Thread Grigory Smolkin
Hello! On 11/29/19 11:07 AM, Artur Zakirov wrote: If Anastasia doesn't mind I'd like to send new version of the patch. On 2019/11/28 12:29, Artur Zakirov wrote: On 2019/11/27 13:22, Michael Paquier wrote: Yeah, the actual take is if we want to make the frontend code more complicated with a

Re: pg_upgrade fails with non-standard ACL

2019-11-29 Thread Artur Zakirov
If Anastasia doesn't mind I'd like to send new version of the patch. On 2019/11/28 12:29, Artur Zakirov wrote: On 2019/11/27 13:22, Michael Paquier wrote: Yeah, the actual take is if we want to make the frontend code more complicated with a large set of SQL queries to check that each object

Re: pg_upgrade fails with non-standard ACL

2019-11-27 Thread Artur Zakirov
On 2019/11/27 13:22, Michael Paquier wrote: On Wed, Nov 27, 2019 at 11:35:14AM +0900, Artur Zakirov wrote: Other approach is similar to Anastasia's patch, which is scanning pg_proc, pg_class, pg_attribute and others to get modified ACL's and compare it with initial ACL from pg_init_privs. Next

Re: pg_upgrade fails with non-standard ACL

2019-11-26 Thread Michael Paquier
On Wed, Nov 27, 2019 at 11:35:14AM +0900, Artur Zakirov wrote: > I've started to implement new backend function similar to > pg_describe_object() and tried to make new version of the patch. But I'm > wondering now if it is possible to backpatch new functions to older > Postgres releases?

Re: pg_upgrade fails with non-standard ACL

2019-11-26 Thread Artur Zakirov
Thank you for reviews! On 2019/11/21 17:53, Michael Paquier wrote: On Fri, Nov 15, 2019 at 11:30:02AM +0300, Grigory Smolkin wrote: On 11/9/19 5:26 AM, Michael Paquier wrote: Another question I have: do we need to care more about other extra ACLs applied to other object types? For example a

Re: pg_upgrade fails with non-standard ACL

2019-11-24 Thread Michael Paquier
On Thu, Nov 21, 2019 at 05:53:16PM +0900, Michael Paquier wrote: > Not arguing against the fact that it is useful, but I'd think that it > is a two-step process, where we need to understand what logic needs to > be in the backend or some frontend: > 1) Warn properly about the objects involved,

Re: pg_upgrade fails with non-standard ACL

2019-11-21 Thread Michael Paquier
On Fri, Nov 15, 2019 at 11:30:02AM +0300, Grigory Smolkin wrote: > On 11/9/19 5:26 AM, Michael Paquier wrote: >> 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

Re: pg_upgrade fails with non-standard ACL

2019-11-15 Thread Grigory Smolkin
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

Re: pg_upgrade fails with non-standard ACL

2019-11-08 Thread Michael Paquier
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.

Re: pg_upgrade fails with non-standard ACL

2019-11-08 Thread Michael Paquier
On Mon, Oct 28, 2019 at 05:40:44PM +0300, Anastasia Lubennikova wrote: > I added more comments and updated the error message. > Please, feel free to fix them, if you have any suggestions. I have begun looking at this one. + /* REVOKE command must be executed in corresponding database */ + if

Re: pg_upgrade fails with non-standard ACL

2019-10-28 Thread Anastasia Lubennikova
08.10.2019 17:08, Stephen Frost wrote: I attached the updated version. Now it prints a better error message and generates an SQL script instead of multiple warnings. The attached test script shows that. Have you tested this with extensions, where the user has changed the privileges on the

Re: pg_upgrade fails with non-standard ACL

2019-10-08 Thread Stephen Frost
Greetings, * Anastasia Lubennikova (a.lubennik...@postgrespro.ru) wrote: > Cool. It seems that everyone agrees on this patch. Thanks for working on this, I took a quick look over it and I do have some concerns. > I attached the updated version. Now it prints a better error message > and

Re: pg_upgrade fails with non-standard ACL

2019-10-04 Thread Anastasia Lubennikova
27.09.2019 15:51, Bruce Momjian wrote: On Fri, Sep 27, 2019 at 04:22:15PM +0900, Michael Paquier wrote: On Thu, Sep 26, 2019 at 04:19:38PM -0400, Bruce Momjian wrote: On Thu, Sep 26, 2019 at 05:16:19PM -0300, Alvaro Herrera wrote: On 2019-Sep-26, Bruce Momjian wrote: Well, right now,

Re: pg_upgrade fails with non-standard ACL

2019-09-27 Thread Bruce Momjian
On Fri, Sep 27, 2019 at 04:22:15PM +0900, Michael Paquier wrote: > On Thu, Sep 26, 2019 at 04:19:38PM -0400, Bruce Momjian wrote: > > On Thu, Sep 26, 2019 at 05:16:19PM -0300, Alvaro Herrera wrote: > >> On 2019-Sep-26, Bruce Momjian wrote: > >>> Well, right now, pg_upgrade --check succeeds, but

Re: pg_upgrade fails with non-standard ACL

2019-09-27 Thread Michael Paquier
On Thu, Sep 26, 2019 at 04:19:38PM -0400, Bruce Momjian wrote: > On Thu, Sep 26, 2019 at 05:16:19PM -0300, Alvaro Herrera wrote: >> On 2019-Sep-26, Bruce Momjian wrote: >>> Well, right now, pg_upgrade --check succeeds, but the upgrade fails. I >>> am proposing, at a minimum, that pg_upgrade

Re: pg_upgrade fails with non-standard ACL

2019-09-26 Thread Bruce Momjian
On Thu, Sep 26, 2019 at 05:16:19PM -0300, Alvaro Herrera wrote: > On 2019-Sep-26, Bruce Momjian wrote: > > Well, right now, pg_upgrade --check succeeds, but the upgrade fails. I > > am proposing, at a minimum, that pg_upgrade --check fails in such cases, > > Agreed, that should be a minimum fix.

Re: pg_upgrade fails with non-standard ACL

2019-09-26 Thread Alvaro Herrera
On 2019-Sep-26, Bruce Momjian wrote: > On Wed, Sep 11, 2019 at 06:25:38PM -0300, Álvaro Herrera wrote: > > On 2019-Aug-21, Bruce Momjian wrote: > > > > > > 1) How exactly should we report this incompatibility to a user? > > > > I think it's fine to leave the warnings and also write some hint for

Re: pg_upgrade fails with non-standard ACL

2019-09-26 Thread Bruce Momjian
On Wed, Sep 11, 2019 at 06:25:38PM -0300, Álvaro Herrera wrote: > On 2019-Aug-21, Bruce Momjian wrote: > > > > 1) How exactly should we report this incompatibility to a user? > > > I think it's fine to leave the warnings and also write some hint for the > > > user by analogy with other checks. >

Re: pg_upgrade fails with non-standard ACL

2019-09-11 Thread Alvaro Herrera from 2ndQuadrant
On 2019-Aug-21, Bruce Momjian wrote: > > 1) How exactly should we report this incompatibility to a user? > > I think it's fine to leave the warnings and also write some hint for the > > user by analogy with other checks. > > "Reset ACL on the problem functions to default in the old cluster to > >

Re: pg_upgrade fails with non-standard ACL

2019-09-11 Thread Alvaro Herrera from 2ndQuadrant
Stephen, On 2019-Aug-20, Stephen Frost wrote: > Will try to take a look at the actual patch later today. Any word on that review? -- Álvaro Herrerahttps://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

Re: pg_upgrade fails with non-standard ACL

2019-08-21 Thread Bruce Momjian
On Tue, Aug 20, 2019 at 04:38:18PM +0300, Anastasia Lubennikova wrote: > > Solving this in pg_upgrade does seem like it's probably the better > > approach rather than trying to do it in pg_dump. Unfortunately, that > > likely means that all we can do is have pg_upgrade point out to the user > >

Re: pg_upgrade fails with non-standard ACL

2019-08-20 Thread Stephen Frost
Greetings, * Anastasia Lubennikova (a.lubennik...@postgrespro.ru) wrote: > 14.08.2019 3:28, Stephen Frost wrote: > >* Bruce Momjian (br...@momjian.us) wrote: > >>As much as it would be nice if the release notes covered all that, and > >>we updated pg_upgrade to somehow handle them, it just isn't

Re: pg_upgrade fails with non-standard ACL

2019-08-20 Thread Anastasia Lubennikova
14.08.2019 3:28, Stephen Frost wrote: * Bruce Momjian (br...@momjian.us) wrote: As much as it would be nice if the release notes covered all that, and we updated pg_upgrade to somehow handle them, it just isn't realistic. As we can see here, the problems often take years to show up, and even

Re: pg_upgrade fails with non-standard ACL

2019-08-13 Thread Bruce Momjian
On Tue, Aug 13, 2019 at 08:28:12PM -0400, Stephen Frost wrote: > Getting it to be at check time would certainly be a great improvement. > > Solving this in pg_upgrade does seem like it's probably the better > approach rather than trying to do it in pg_dump. Unfortunately, that > likely means

Re: pg_upgrade fails with non-standard ACL

2019-08-13 Thread Stephen Frost
Greetings, * Bruce Momjian (br...@momjian.us) wrote: > On Tue, Aug 13, 2019 at 07:04:42PM +0300, Anastasia Lubennikova wrote: > > Maybe, as a compromise, we can reset grants to default for all changed > > functions > > and also generate a script that will allow a user to preserve privileges of >

Re: pg_upgrade fails with non-standard ACL

2019-08-13 Thread Bruce Momjian
On Tue, Aug 13, 2019 at 07:04:42PM +0300, Anastasia Lubennikova wrote: > > In an ideal world, it seems like we'd make a judgement call and arrange > > to pull the privileges across when we can do so without granting the > > user privileges beyond those that were intended, and otherwise we'd > >

Re: pg_upgrade fails with non-standard ACL

2019-08-13 Thread Anastasia Lubennikova
29.07.2019 18:37, Stephen Frost wrote: Greetings, * Tom Lane (t...@sss.pgh.pa.us) wrote: Bruce Momjian writes: On Thu, Jul 18, 2019 at 06:53:12PM +0300, Anastasia Lubennikova wrote: pg_upgrade from 9.6 fails if old cluster had non-standard ACL on pg_catalog functions that have changed

Re: pg_upgrade fails with non-standard ACL

2019-07-29 Thread Stephen Frost
Greetings, * Tom Lane (t...@sss.pgh.pa.us) wrote: > Bruce Momjian writes: > > On Thu, Jul 18, 2019 at 06:53:12PM +0300, Anastasia Lubennikova wrote: > >> pg_upgrade from 9.6 fails if old cluster had non-standard ACL > >> on pg_catalog functions that have changed between versions, > >> for

Re: pg_upgrade fails with non-standard ACL

2019-07-27 Thread Tom Lane
Bruce Momjian writes: > On Thu, Jul 18, 2019 at 06:53:12PM +0300, Anastasia Lubennikova wrote: >> pg_upgrade from 9.6 fails if old cluster had non-standard ACL >> on pg_catalog functions that have changed between versions, >> for example pg_stop_backup(boolean). > Uh, wouldn't this affect any

Re: pg_upgrade fails with non-standard ACL

2019-07-27 Thread Bruce Momjian
On Thu, Jul 18, 2019 at 06:53:12PM +0300, Anastasia Lubennikova wrote: > pg_upgrade from 9.6 fails if old cluster had non-standard ACL > on pg_catalog functions that have changed between versions, > for example pg_stop_backup(boolean). > > Error: > > pg_restore: creating ACL "pg_catalog.FUNCTION