Re: Pg_upgrade and toast tables bug discovered

On Wed, Jul 16, 2014 at 10:19 AM, Bruce Momjian <bruce(at)momjian(dot)us> wrote:
> On Mon, Jul 14, 2014 at 09:40:33PM +0200, Andres Freund wrote:
>> On 2014-07-11 09:55:34 -0400, Bruce Momjian wrote:
>> > On Fri, Jul 11, 2014 at 09:48:06AM -0400, Bruce Momjian wrote:
>> > > > Uh, why does this need to be in ALTER TABLE? Can't this be part of
>> > > > table creation done by pg_dump?
>> > >
>> > > Uh, I think you need to read the thread. We have to delay the toast
>> > > creation part so we don't use an oid that will later be required by
>> > > another table from the old cluster. This has to be done after all
>> > > tables have been created.
>> > >
>> > > We could have pg_dump spit out those ALTER lines at the end of the dump,
>> > > but it seems simpler to do it in pg_upgrade.
>> > >
>> > > Even if we have pg_dump create all the tables that require pre-assigned
>> > > TOAST oids first, then the other tables that _might_ need a TOAST table,
>> > > those later tables might create a toast oid that matches a later
>> > > non-TOAST-requiring table, so I don't think that fixes the problem.
>> >
>> > What would be nice is if I could mark just the tables that will need
>> > toast tables created in that later phase (those tables that didn't have
>> > a toast table in the old cluster, but need one in the new cluster).
>> > However, I can't see where to store that or how to pass that back into
>> > pg_upgrade. I don't see a logical place in pg_class to put it.
>>
>> This seems overengineered. Why not just do
>> SELECT pg_upgrade.maintain_toast(oid) FROM pg_class WHERE relkind = 'r';
>>
>> and in maintain_toast() just call AlterTableCreateToastTable()?
>
> I didn't think you could call into backend functions like that, and if I
> did, it might break something in the future.

It would be impossible to write meaningful code in extensions if that
were true. Yeah, it could break in the future, but it's not
particularly likely, the regression tests should catch it if it
happens, and it's no worse a risk here than in any other extension
code which does this.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company