On 2018-08-21 17:04:41 +0200, Peter Eisentraut wrote: > On 20/08/2018 15:34, Tom Lane wrote: > > Andres Freund <and...@anarazel.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