pg_upgrade check for invalid role-specific default config

2021-04-12 Thread Charlie Hornsby
Hi all,

While troubleshooting a failed upgrade from v11 -> v12 I realised I had
encountered a bug previously reported on the pgsql-bugs mailing list:

#14242 Role with a setconfig "role" setting to a nonexistent role causes
pg_upgrade to fail

https://www.postgresql.org/message-id/20160711223641.1426.86096%40wrigleys.postgresql.org

To quote the previous report:

> It is possible to modify the "role" setting in setconfig in the
> pg_db_role_setting table such that it points to a nonexistent role.  When
> this is the case, restoring the output of pg_dumpall will fail due to the
> missing role.

> Steps to reproduce:

> 1. As superuser, execute "create role foo with login password 'test'"
> 2. As foo, execute "alter role foo set role = 'foo'"
> 3. As superuser, execute "alter role foo rename to bar"
> a. At this point, the setconfig entry in pg_db_role_setting for
> 'bar' will contain '{role=foo}', which no longer exists
> 4. Execute pg_upgrade with the recommended steps in
> https://www.postgresql.org/docs/current/static/pgupgrade.html

> During pg_upgrade (more specifically, during the restore of the output from
> pg_dumpall), the "ALTER ROLE "bar" SET "role" TO 'foo'" command generated
> will fail with "ERROR: role "foo" does not exist".

> This issue was identified by Jordan Lange and Nathan Bossart.

The steps in the original report reproduce the problem on all currently
supported pg versions. I appreciate that the invalid role-specific default 
settings are ultimately self-inflicted by the user, but as a third-party 
performing the upgrade this caught me by surprise.

Since it is possible to write a query to identify these cases, would there
be appetite for me to submit a patch to add a check for this to 
pg_upgrade?

First time mailing list user here so many apologies for any missteps I have
made in this message.

Best regards,
Charlie Hornsby



Re: pg_upgrade check for invalid role-specific default config

2021-04-13 Thread Charlie Hornsby
Tom wrote:
> I do find it interesting that we now have two reports of somebody
> doing "ALTER ROLE SET role = something".  In the older thread,
> I was skeptical that that had any real use-case, so I wonder if
> Charlie has a rationale for having done that.

Unfortunately I haven't heard back from the original developer
who set up this role configuration, but if I do then I will share
their intentions.  In any case the invalid configuration had been
removed from every other role except one (certainly by mistake)
which lead to me rediscovering this issue.

I tested the above patch with the invalid data locally and it avoids
the restore error that we ran into previously.  Also it requires no
intervention to progress with pg_upgrade unlike my initial idea of
adding an check, so it is definitely simpler from a user perspective.

Thank you for taking a deep look into this and finding a better
solution.

Best regards,
Charlie Hornsby