Re: Pg_upgrade and toast tables bug discovered

On Wed, Jul 9, 2014 at 10:13:17AM -0400, Robert Haas wrote:
> >> Uh, I guess we could write some code that iterates over all tables and
> >> finds the tables that should have TOAST tables, but don't (because
> >> binary-upgrade backend mode suppressed their creation), and adds them.
> >>
> >> However, that would be a lot of code and might be risky to backpatch.
> >> The error is so rare I am not sure it is worth it. I tried to create
> >> the failure case and it was very difficult, requiring me to create the
> >> problem table first, then some dummy stuff to get the offsets right so
> >> the oid would collide.
> >>
> >> Based on the rareness of the bug, I am not sure it is worth it, but if
> >> it is, it would be something only for 9.4 (maybe) and 9.5, not something
> >> to backpatch.
> >
> > Another idea would be to renumber empty toast tables that conflict
> > during new toast table creation, when in binary upgrade mode. Since the
> > files are always empty in binary upgrade mode, you could just create a
> > new toast table, repoint the old table to use (because it didn't ask for
> > a specific toast table oid because it didn't need one), and then use
> > that one for the table that actually requested the oid. However, if one
> > is a heap (zero size), and one is an index (8k size), it wouldn't work
> > and you would have to recreate the file system file too.
> >
> > That seems a lot more localized than the other approaches.
>
> To me, that sounds vastly more complicated and error-prone than
> forcing the TOAST tables to be added in a second pass as Andres
> suggested.
>
> But I just work here.

Agreed. I am now thinking we could harness the code that already exists
to optionally add a TOAST table as part of ALTER TABLE ADD COLUMN. We
would just need an entry point to call it from pg_upgrade, either via an
SQL command that checks (and hopefully doesn't do anything else), or a C
function that does it, e.g. VACUUM would be trivial to run on every
database, but I don't think it tests that; is _could_ in binary_upgrade
mode. However, the idea of having a C function plug into the guts of
the server and call internal functions makes me uncomforable.

The code already called via pg_restore would only need to suppress TOAST
table creation if a heap oid is supplied but a TOAST table oid is not.

--
Bruce Momjian <bruce(at)momjian(dot)us> http://momjian.us
EnterpriseDB http://enterprisedb.com

+ Everyone has their own god. +