Re: pg_upgrade failing from 9.3 to 9.4 because "template0" already exists

* Tom Lane (tgl(at)sss(dot)pgh(dot)pa(dot)us) wrote:
> Stephen Frost <sfrost(at)snowman(dot)net> writes:
> > * Tom Lane (tgl(at)sss(dot)pgh(dot)pa(dot)us) wrote:
> >> Perhaps pg_upgrade should deliberately ignore template0 regardless of
> >> datallowconn? And/or we should hard-wire that into pg_dumpall?
>
> > My thinking would be that pg_dumpall should be hard-wired for template0
> > (just like it is for template1..) and that we should *not* be excluding
> > databases that are marked as datallowconn = false.. That said, it's not
> > clear to me what to do there instead. Maybe throw an error or a
> > warning? The point of pg_dumpall is to dump *all* the databases and at
> > least the manpage doesn't appear to say anything about "but ignores
> > databases with datallowconn = false".
>
> I think pg_upgrade and pg_dumpall may be two different use-cases.

Perhaps..

> pg_upgrade should definitely throw a hard error if there are any
> non-template0 databases that it can't connect to, because the alternative
> is losing such databases during the upgrade. I'm not sure that the
> argument is so black-and-white for pg_dumpall, though. Nobody's ever
> complained about it skipping unconnectable databases, and that behavior
> has been there since we invented datallowconn (cf commit 2cf48ca04bf599).

Technically, there haven't been any complaints about either pg_dumpall's
behavior in this regard, or pg_upgrade's, but pg_upgrade's post-upgrade
scripts would happily remove any databases which were marked as
'datallowconn = false' and that scares the daylights out of me. To that
end, I'd suggest patching (and back-patching) pg_upgrade to check early
on that:

template0 is set to 'datallowconn = false'

AND

all databases except template0 are set to 'datallowconn = true'

The first is required or anyone who has done that will get the funny
error that started this thread and things won't work anyway, but I
believe the latter is also necessary to patch and back-patch as it could
lead to data loss. It's not a high potential as, hopefully, people will
check first, but I can imagine a hosting provider or environments where
there are lots of independent clusters not catching this issue in their
testing, only to discover someone set their database to 'datallowconn =
false' for whatever reason and now that database is gone...

Thanks!

Stephen