Re: ALTER TABLE on system catalogs

On 2018-08-21 17:04:41 +0200, Peter Eisentraut wrote:
> On 20/08/2018 15:34, Tom Lane wrote:
> > Andres Freund <andres(at)anarazel(dot)de> writes:
> >> On 2018-08-20 14:38:25 +0200, Peter Eisentraut wrote:
> >>> Do you have an alternative in mind?
> >
> >> One is to just not do anything. I'm not sure I'm on board with the goal
> >> of changing things to make DDL on system tables more palatable.
> >
> > FWIW, I agree with doing as little as possible here. I'd be on board
> > with Andres' suggestion of just swapping the two code stanzas so that
> > the no-op case doesn't error out. As soon as you go beyond that, you
> > are in wildly unsafe and untested territory.
>
> That doesn't solve the original problem, which is being able to set
> reloptions on pg_attribute, because pg_attribute doesn't have a toast
> table but would need one according to existing rules.

I still think it's wrong to work around this than to actually fix the
issue with pg_attribute not having a toast table.

> 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).

> @@ -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.

Greetings,

Andres Freund