Re: alter user/role CURRENT_USER

From: Kyotaro HORIGUCHI <horiguchi(dot)kyotaro(at)lab(dot)ntt(dot)co(dot)jp>
To: alvherre(at)2ndquadrant(dot)com
Cc: tgl(at)sss(dot)pgh(dot)pa(dot)us, adam(dot)brightwell(at)crunchydatasolutions(dot)com, sfrost(at)snowman(dot)net, marti(at)juffo(dot)org, rushabh(dot)lathia(at)gmail(dot)com, pgsql-hackers(at)postgresql(dot)org
Subject: Re: alter user/role CURRENT_USER
Date: 2014-11-14 08:39:33
Message-ID: 20141114.173933.243750776.horiguchi.kyotaro@lab.ntt.co.jp
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Hi, this is revised version.

> Kyotaro HORIGUCHI wrote:
>
> > - Storage for new information
> >
> > The new struct NameId stores an identifier which telling what it
> > logically is using the new enum NameIdTypes.
>
> I think NameId is a bad name for this. My point is that NameId, as it
> stands, might be a name for anything, not just a role; and the object it
> identifies is not an Id either. Maybe RoleSpec?

Yeah! I felt it no good even if it were a generic type for
various "Name of something or its oid". RoleSpec sounds much better.

> Do we need a public_ok
> argument to get_nameid_oid() (get a better name for this function too)

Maybe get_rolespec_oid() as a name ofter its parameter type?

> so that callers don't have to check for InvalidOid argument? I think
> the arrangement you propose is not very convenient; it'd be best to
> avoid duplicating the check for InvalidOid in all callers of the new
> function, particularly where there was no check before.

I agree that It'd be better keeping away from duplicated
InvalidOid checks, but public_ok seems a bit myopic. Since
there's no reasonable border between functions accepting 'public'
and others, such kind of solution would not be reasonable..

What about checking it being a PUBLIC or not *before* calling
get_rolespec_oid()?

The attached patch modified in the following points.

- rename NameId to RoleSpec and NameIdType to RoleSpecTypes.
- rename get_nameid_oid() to get_rolespec_oid().
- rename roleNamesToIds() to roleSpecsToIds().
- some struct members are changed such as authname to authrole.
- check if rolespec is "public" or not before calling get_rolespec_oid()
- ExecAlterDefaultPrivilegesStmt and ExecuteGrantStmt does
slightly different things about ACL_ID_PUBLIC but I unified it
to the latter.
- rebased to the current master

regards,

--
Kyotaro Horiguchi
NTT Open Source Software Center

CreateStmt->authrole = NULL => ?

Attachment Content-Type Size
0001-ALTER-USER-CURRENT_USER-v3.patch text/x-patch 44.9 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message David Rowley 2014-11-14 09:02:49 Re: using custom scan nodes to prototype parallel sequential scan
Previous Message Michael Paquier 2014-11-14 08:31:08 Re: WAL format and API changes (9.5)