Обсуждение: Re: [GENERAL] pg_upgrade from 9.0.7 to 9.1.3: duplicate key pg_authid_oid_index

On Fri, Jun 01, 2012 at 09:52:59AM -0400, Tom Lane wrote:
> Bryan Murphy <bmurphy1976@gmail.com> writes:
> > The old 9.0 cluster was created by ubuntu.  In this cluster there was an
> > ubuntu user with an oid of 10 and a postgres user with an oid of 16386.
>
> > The new 9.1 cluster was created with a custom build of postgres 9.1. This
> > did not have an ubuntu user, and it had a postgres user with an oid of 10.
>
> OID 10 is the bootstrap superuser, which is created with the name of the
> operating system user that ran initdb.  So the above does not sound like
> anything to do with custom vs stock builds, but with who did initdb.
>
> It seems that pg_upgrade needs a check to make sure that the bootstrap
> superuser is named the same in old and new clusters.

[ Thread moved to hackers.]

OK, I have studied this.  First we preserve pg_authid.oid because oids
are stored in pg_largeobject_metadata.  Second, we dumpall all users,
even the install user because (from pg_dumpall.c):

         * We dump CREATE ROLE followed by ALTER ROLE to ensure that the role
         * will acquire the right properties even if it already exists (ie, it
         * won't hurt for the CREATE to fail).  This is particularly important
         * for the role we are connected as, since even with --clean we will
         * have failed to drop it.

So, pg_upgrade has to strip out restoring the install user because that
would cause an error on restore.  That is done in
dump.c::split_old_dump().

The problem is if the old and new install users have different oids, as
the reporter verified.

The attached patch adds checks to verify the the old/new servers have
the same install-user oid.

--
  Bruce Momjian  <bruce@momjian.us>        http://momjian.us
  EnterpriseDB                             http://enterprisedb.com

  + It's impossible for everything to be true. +

Bruce Momjian <bruce@momjian.us> writes:
> On Fri, Jun 01, 2012 at 09:52:59AM -0400, Tom Lane wrote:
>> It seems that pg_upgrade needs a check to make sure that the bootstrap
>> superuser is named the same in old and new clusters.

> The attached patch adds checks to verify the the old/new servers have
> the same install-user oid.

That may or may not be a useful check to make, but it's got
approximately nothing to do with what I was complaining about.

In particular, supposing that the user has given you a username that
isn't the bootstrap superuser in the new cluster, this patch is not
going to stop the update script from failing.  Because the script is
then going to try to replace the bootstrap superuser, and that is
certainly going to give an error.

I see the point of worrying about the install user as well as the
bootstrap superuser, but wouldn't it be best to insist they be the same?
Particularly in the new cluster, where if they aren't the same it means
the user has manually created at least one role in the new cluster,
which is likely to lead to OID conflicts or worse.

Furthermore, if the bootstrap superusers aren't named the same, your
patch fails to handle the original complaint.  In the case the
OP mentioned, the old cluster hadOID 10: "ubuntu"some user-defined OID: "postgres"
and the new cluster hadOID 10: "postgres"
If the user tells pg_upgrade to use username postgres, your check will
not fail AFAICS, but nonetheless things are going to be messed up after
the upgrade, because some objects and privileges that used to belong to
the bootstrap superuser will now belong to a non-default superuser,
whereas what used to belong to the non-default superuser will now belong
to the bootstrap superuser.  That cannot be thought desirable.  For one
reason, in the old installation the postgres role could have been
dropped (possibly after dropping a few non-builtin objects) whereas the
"ubuntu" role was pinned.  In the new installation, "postgres" is pinned
and "ubuntu" won't be.

I think the checks that are actually needed here are (1) bootstrap
superusers are named the same, and (2) there are no roles other than the
bootstrap superuser in the new cluster.
        regards, tom lane

On Sat, Jun 02, 2012 at 05:10:03PM -0400, Tom Lane wrote:
> Bruce Momjian <bruce@momjian.us> writes:
> > On Fri, Jun 01, 2012 at 09:52:59AM -0400, Tom Lane wrote:
> >> It seems that pg_upgrade needs a check to make sure that the bootstrap
> >> superuser is named the same in old and new clusters.
>
> > The attached patch adds checks to verify the the old/new servers have
> > the same install-user oid.
>
> That may or may not be a useful check to make, but it's got
> approximately nothing to do with what I was complaining about.
>
> In particular, supposing that the user has given you a username that
> isn't the bootstrap superuser in the new cluster, this patch is not
> going to stop the update script from failing.  Because the script is
> then going to try to replace the bootstrap superuser, and that is
> certainly going to give an error.
>
> I see the point of worrying about the install user as well as the
> bootstrap superuser, but wouldn't it be best to insist they be the same?
> Particularly in the new cluster, where if they aren't the same it means
> the user has manually created at least one role in the new cluster,
> which is likely to lead to OID conflicts or worse.
>
> Furthermore, if the bootstrap superusers aren't named the same, your
> patch fails to handle the original complaint.  In the case the
> OP mentioned, the old cluster had
>     OID 10: "ubuntu"
>     some user-defined OID: "postgres"
> and the new cluster had
>     OID 10: "postgres"
> If the user tells pg_upgrade to use username postgres, your check will
> not fail AFAICS, but nonetheless things are going to be messed up after
> the upgrade, because some objects and privileges that used to belong to
> the bootstrap superuser will now belong to a non-default superuser,
> whereas what used to belong to the non-default superuser will now belong
> to the bootstrap superuser.  That cannot be thought desirable.  For one
> reason, in the old installation the postgres role could have been
> dropped (possibly after dropping a few non-builtin objects) whereas the
> "ubuntu" role was pinned.  In the new installation, "postgres" is pinned
> and "ubuntu" won't be.
>
> I think the checks that are actually needed here are (1) bootstrap
> superusers are named the same, and (2) there are no roles other than the
> bootstrap superuser in the new cluster.

You are right that it is more complex than I stated, but given the
limited feedback I got on the pg_upgrade/plplython, I figured people
didn't want to hear the details.  Here they are:

There are three failure modes for pg_upgrade:

1.  check failure
2.  schema restore failure
3.  silent failure/corruption

Of course, the later items are worse than the earlier ones.  The
reporter got a "schema restore failure" while still following the
pg_upgrade instructions.  My initial patch changed that #2 error to a #1
error.  Tom is right that creating users in the new cluster (against
instructions), can still generate a #2 error if a new/old pg_authid.oid
match, and they are not the install user, but seeing that is something
that is against the instructions, I was going to leave that as a #2.

However, since Tom feels we should check that and make it a #1 failure,
I have added that test to the attached patch.

--
  Bruce Momjian  <bruce@momjian.us>        http://momjian.us
  EnterpriseDB                             http://enterprisedb.com

  + It's impossible for everything to be true. +

On Mon, Jun 04, 2012 at 10:16:45AM -0400, Bruce Momjian wrote:
> > I think the checks that are actually needed here are (1) bootstrap
> > superusers are named the same, and (2) there are no roles other than the
> > bootstrap superuser in the new cluster.
> 
> You are right that it is more complex than I stated, but given the
> limited feedback I got on the pg_upgrade/plplython, I figured people
> didn't want to hear the details.  Here they are:
> 
> There are three failure modes for pg_upgrade:
> 
> 1.  check failure
> 2.  schema restore failure
> 3.  silent failure/corruption
> 
> Of course, the later items are worse than the earlier ones.  The
> reporter got a "schema restore failure" while still following the
> pg_upgrade instructions.  My initial patch changed that #2 error to a #1
> error.  Tom is right that creating users in the new cluster (against
> instructions), can still generate a #2 error if a new/old pg_authid.oid
> match, and they are not the install user, but seeing that is something
> that is against the instructions, I was going to leave that as a #2.

Applied and back-patched to Postgres 9.1.

--  Bruce Momjian  <bruce@momjian.us>        http://momjian.us EnterpriseDB
http://enterprisedb.com
 + It's impossible for everything to be true. +