Re: logical changeset generation v6.6

On 2013-11-12 13:18:19 -0500, Robert Haas wrote:
> On Tue, Nov 12, 2013 at 12:50 PM, Andres Freund <andres(at)2ndquadrant(dot)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 :/

I'd suggest renaming the current IsSystemRelation() to your
IsRelationInSystemNamespace() and add IsCatalogRelation() for the new
meaning, so we are sure to break old users.

Let me come up with something like that.

> (Maybe the
> user-catalog-tables patch will modify this test; I'm not sure, but if
> this needs to work differently it seems that it should be conditional
> on that, not what schema the table lives in.)

No, they shouldn't change that. We might want to allow such locking
semantics at some points, but that'd be a separate patch.

Greetings,

Andres Freund

--
Andres Freund http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services