Re: Re: [COMMITTERS] pgsql: Support comments on FOREIGN DATA WRAPPER and SERVER objects.

On Mon, Apr 4, 2011 at 6:49 AM, Shigeru HANADA
<hanada(at)metrosystems(dot)co(dot)jp> wrote:
> 1) Who can comment on a user mapping?
> Basically only the owner can comment on a object, but user mappings
> don't have owner.  So following rules for ALTER/DROP seems good
> because they are similarly allowed to only owner.  In addition to
> server's owner, a user can perform ALTER/DROP USER MAPPING if target
> mapping is his own user's and USAGE privilege on the server has been
> granted.  This means that mappings for PUBLIC can be commented by only
> server's owner.  Is this spec reasonable?

I guess so. The existing rules for user mapping permissions are not
really what I would have expected, but there doesn't seem to be any
particular reason to deviate from them here. I do think however that
we should try NOT to duplicate the logic in user_mapping_ddl_aclcheck
into multiple places, as someone will certainly screw that up down the
road. If we're going to have complex logic like this we need to at
least make sure that we only have one copy of it.

>> 2) How to specify user name of the target mapping
> ALTER/DROP USER MAPPING also accept USER and CURRENT_USER as current
> user.  This syntax seems suitable for COMMENT ON USER MAPPING too for
> consistency and usability.

OK, sounds fine. But what does it actually mean when you say
{ALTER|DROP|COMMENT ON} USER MAPPING FOR USER SERVER servername? I
see some code to handle CURRENT_USER, but I must be missing the
special-casing for USER. It's not documented, either. :-(

> 3) Omitting ACL framework support
> ISTM that full-support of ACL framework is not necessary for USER
> MAPPING because USER MAPPING has no GRANT/REVOKE statements.
> COMMENT ON USER MAPPING patch works fine, but some oversight might be
> here.

I agree.

BTW, I think you can merge patches 0001 to 0004 into a single patch.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company