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