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
differentthings 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 => ?
