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