Skip site navigation (1) Skip section navigation (2)

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

From: Robert Haas <robertmhaas(at)gmail(dot)com>
To: Shigeru HANADA <hanada(at)metrosystems(dot)co(dot)jp>
Cc: Shigeru Hanada <shigeru(dot)hanada(at)gmail(dot)com>, Thom Brown <thom(at)linux(dot)com>, pgsql-hackers(at)postgresql(dot)org
Subject: Re: Re: [COMMITTERS] pgsql: Support comments on FOREIGN DATA WRAPPER and SERVER objects.
Date: 2011-04-04 16:47:18
Message-ID: BANLkTimyETAP=ZVxv4YPuSsLctvJX8eeWw@mail.gmail.com (view raw or flat)
Thread:
Lists: pgsql-committerspgsql-hackers
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

In response to

Responses

pgsql-hackers by date

Next:From: Robert HaasDate: 2011-04-04 16:50:03
Subject: Re: GSoC proposal: Fast GiST index build
Previous:From: Alexander KorotkovDate: 2011-04-04 16:46:06
Subject: Re: GSoC proposal: Fast GiST index build

pgsql-committers by date

Next:From: Robert HaasDate: 2011-04-04 17:23:54
Subject: pgsql: Include pid in pg_lock_status() results even for SIREAD locks.
Previous:From: Alvaro HerreraDate: 2011-04-04 16:05:30
Subject: pgsql: Duplicate expansion of "direction" from FETCH's synopsis intoMO

Privacy Policy | About PostgreSQL
Copyright © 1996-2014 The PostgreSQL Global Development Group