On 2013-11-12 19:24:39 +0100, Andres Freund wrote:
> On 2013-11-12 13:18:19 -0500, Robert Haas wrote:
> > On Tue, Nov 12, 2013 at 12:50 PM, Andres Freund <andres@2ndquadrant.com> wrote:
> > > Completely agreed. As evidenced by the fact that the current change
> > > doesn't update all relevant comments & code. I wonder if we shouldn't
> > > leave the function the current way and just add a new function for the
> > > new behaviour.
> > > The hard thing with that would be coming up with a new
> > > name. IsSystemRelationId() having a different behaviour than
> > > IsSystemRelation() seems strange to me, so just keeping that and
> > > adapting the callers seems wrong to me.
> > > IsInternalRelation()? IsCatalogRelation()?
> >
> > Well, I went through and looked at the places that were affected by
> > this and I tend to think that most places will be happier with the new
> > definition.
>
> I agree that many if not most want the new definition.
>
> > If there are call sites that want the existing test, maybe we should
> > have IsRelationInSystemNamespace() for that, and reserve
> > IsSystemRelation() for the test as to whether it's a bona fide system
> > catalog.
>
> The big reason that I think we do not want the new behaviour for all is:
>
> * NB: TOAST relations are considered system relations by this test
> * for compatibility with the old IsSystemRelationName function.
> * This is appropriate in many places but not all. Where it's not,
> * also check IsToastRelation.
>
> the current state of things would allow to modify toast relations in
> some places :/
So, I think I found a useful defintion of IsSystemRelation() that fixes
many of the issues with moving relations to pg_catalog: Continue to
treat all pg_toast.* relations as system tables, but only consider
initdb created relations in pg_class.
I've then added IsCatalogRelation() which has a narrower definition of
system relations, namely, it only counts toast tables if they are a
catalog's toast table.
This allows far more actions on user defined relations moved to
pg_catalog. Now they aren't stuck there anymore and can be renamed,
dropped et al. With one curious exception: We still cannot move a
relation out of pg_catalog.
I've included a hunk to allow creation of indexes on relations in
pg_catalog in heap_create(), indexes on catalog relations are prevented
way above, but maybe that should rather be a separate commit.
What do you think?
Greetings,
Andres Freund
--
Andres Freund http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services
