On Tue, Oct 01, 2019 at 12:10:50AM +0000, Hsu, John wrote:
> get_relkind_objtype(...) was introduced as part of 8b9e9644dc, and it doesn't include
> RELKIND_TOASTVALUE. As a result when a user who has usage rights on schema pg_toast
> and attempts to reindex a table it is not the owner of it fails with the wrong error
> message.
(Adding Peter E. in CC)
Sure. However this implies that the user doing the reindex not only
has ownership of the relation worked on, but is also able to work
directly on the schema pg_toast. Should we really encourage people to
do that with non-superusers?
> It seems get_relkind_objtype(...) is only used as part of aclcheck_error(...)
> I've attached a patch to include RELKIND_TOASTVALUE in get_relkind_objtype.
> Now it fails with the proper error message.
>
> postgres=> \c
> You are now connected to database "postgres" as user "testuser".
> postgres=> REINDEX TABLE pg_toast.pg_toast_16388;
> ERROR: must be owner of table pg_toast_16388
Here is a set of commands to see the failure:
=# CREATE ROLE testuser LOGIN;
=# GRANT USAGE ON SCHEMA pg_toast TO testuser;
\c postgres testuser
=> REINDEX TABLE pg_toast.pg_toast_2609;
ERROR: XX000: unexpected relkind: 116
=> REINDEX INDEX pg_toast.pg_toast_2609_index;
ERROR: 42501: must be owner of index pg_toast_2609_index
LOCATION: aclcheck_error, aclchk.c:3623
As you wrote, get_relkind_objtype() is primarily used for ACL errors,
but we have another set of code paths with get_object_type() which
gets called for a subset of ALTER TABLE commands. So this error can
be triggered in more ways, though you had better not do the following
one:
=# ALTER TABLE pg_toast.pg_toast_1260 rename to popo;
ERROR: XX000: unexpected relkind: 116
The comment about OBJECT_* in get_relkind_objtype() is here because
there is no need for toast objects to have object address support
(there is a test in object_address.sql about that), and ObjectTypeMap
has no mapping OBJECT_* <-> toast table, so the change proposed is not
correct from this perspective.
--
Michael