Re: Include RELKIND_TOASTVALUE in get_relkind_objtype

From: Michael Paquier <michael(at)paquier(dot)xyz>
To: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Cc: "Hsu, John" <hsuchen(at)amazon(dot)com>, "pgsql-hackers(at)postgresql(dot)org" <pgsql-hackers(at)postgresql(dot)org>, Peter Eisentraut <peter(dot)eisentraut(at)2ndquadrant(dot)com>
Subject: Re: Include RELKIND_TOASTVALUE in get_relkind_objtype
Date: 2019-10-04 08:55:40
Message-ID: 20191004085540.GB1829@paquier.xyz
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Thu, Oct 03, 2019 at 09:52:34AM -0400, Tom Lane wrote:
> FWIW, I really dislike this patch, mainly because it is based on the
> assumption (as John said) that get_relkind_objtype is used only
> in aclcheck_error calls. However it's not obvious why that should
> be true, and there certainly is no documentation suggesting that
> it needs to be true. That's mainly because get_relkind_objtype has no
> documentation period, which if you ask me is flat out unacceptable
> for a globally-exposed function. (Same comment about its wrapper
> get_object_type.)

Yes, I agree that the expectations that the caller of this function
can have are hard to guess. So we could tackle this occasion to add
more comments. I could try to come up with a better patch. Or
perhaps you have already your mind on it?

> The patch also falsifies the comment just a few lines away that
>
> /*
> * other relkinds are not supported here because they don't map to
> * OBJECT_* values
> */
>
> without doing anything about that.

That's actually what I was referring to in my previous email.

> I'm inclined to think that we should redefine the charter of
> get_relkind_objtype/get_object_type to be that they'll produce
> some OBJECT_* value for any relkind whatever, on the grounds
> that throwing an error here isn't a particularly useful behavior;
> we'd rather come out with a possibly-slightly-inaccurate generic
> message about a "table". And they need to be documented that way.

This is tempting.

> Alternatively, instead of mapping other relkinds to OBJECT_TABLE,
> we could invent a new enum entry OBJECT_RELATION. There's precedent
> for that in OBJECT_ROUTINE ... but I don't know that we want to
> build out all the other infrastructure for a new ObjectType right now.

I am too lazy to check the thread that led to 8b9e964, but I recall
that Peter wanted to get rid of OBJECT_RELATION because that's
confusing as that's not an purely exclusive object type, and it mapped
with other object types.
--
Michael

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Michael Paquier 2019-10-04 08:57:14 Re: dropping column prevented due to inherited index
Previous Message Michael Paquier 2019-10-04 08:37:21 Re: Two pg_rewind patches (auto generate recovery conf and ensure clean shutdown)