Re: ALTER TABLE on system catalogs

On 21/08/2018 17:24, Andres Freund wrote:
>> Attached is a patch that instead moves those special cases into
>> needs_toast_table(), which was one of the options suggested by Andres.
>> There were already similar checks there, so I guess this makes a bit of
>> sense.
> The big difference is that that then only takes effect when called with
> check=true. The callers without it, importantly NewHeapCreateToastTable
> & NewRelationCreateToastTable, then won't run into this check. But still
> into the error (see below).

I don't follow. The call to needs_toast_table() is not conditional on
the check argument. The check argument only checks that the correct
lock level is passed in.

>> @@ -145,21 +146,6 @@ create_toast_table(Relation rel, Oid toastOid, Oid toastIndexOid,
>> ObjectAddress baseobject,
>> toastobject;
>>
>> - /*
>> - * Toast table is shared if and only if its parent is.
>> - *
>> - * We cannot allow toasting a shared relation after initdb (because
>> - * there's no way to mark it toasted in other databases' pg_class).
>> - */
>> - shared_relation = rel->rd_rel->relisshared;
>> - if (shared_relation && !IsBootstrapProcessingMode())
>> - ereport(ERROR,
>> - (errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
>> - errmsg("shared tables cannot be toasted after initdb")));
> This error check imo shouldn't be removed, but moved down.

We could keep it, but it would probably be dead code since
needs_toast_table() would return false for all shared tables anyway.

--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services