Re: ALTER TABLE on system catalogs

Hi,

On 2018-06-27 22:31:30 +0200, Peter Eisentraut wrote:
> ALTER TABLE on system catalogs is occasionally useful, for example
>
> ALTER TABLE pg_attribute SET (autovacuum_vacuum_scale_factor=0);

> However, this doesn't actually work. The above command produces
>
> ERROR: AccessExclusiveLock required to add toast table.
>
> If it's a shared catalog, it will produce
>
> ERROR: shared tables cannot be toasted after initdb
>
> In other cases it will work but then silently add a TOAST table to a
> catalog, which I think we don't want.
>
> The problem is that for (almost) any ALTER TABLE command, it afterwards
> checks if the just-altered table now needs a TOAST table and tries to
> add it if so, which will either fail or add a TOAST table that we don't
> want.
>
> I propose that we instead silently ignore attempts to add TOAST tables
> to shared and system catalogs after bootstrapping.

That seems like an extremely bad idea to me. I'd rather go ahead and
just add the necessary toast tables than silently violate preconditions
that code assumes are fulfilled.

> This fixes the above
> issues. I have attached a patch for this, and also a test that
> enshrines which system catalogs are supposed to have TOAST tables.
>
> As an alternative, I tried to modify the ALTER TABLE code to avoid the
> try-to-add-TOAST-table path depending on what ALTER TABLE actions were
> done, but that seemed incredibly more complicated and harder to maintain
> in the long run.
>
> (You still need allow_system_table_mods=on for all of this. Perhaps
> that's also worth revisiting, but it's a separate issue.)

I think we should make it harder, not easier to modify system
catalogs. In fact, I think we should require allow_system_table_mods=on
on catalog DML, not just DDL.

I think we can add explicit exceptions - like changing autovac settings
- however.

Greetings,

Andres Freund