Hello,
> Kyotaro,
>
> Food for thought. Couldn't you reduce the following block:
>
> + if (strcmp(stmt->role, "current_user") == 0)
> + {
> + roleid = GetUserId();
> + tuple = SearchSysCache1(AUTHOID, ObjectIdGetDatum(roleid));
> + if (!HeapTupleIsValid(tuple))
> + ereport(ERROR,
> + (errcode(ERRCODE_UNDEFINED_OBJECT),
> + errmsg("roleid %d does not exist", roleid)));
> + }
> + else
> + {
> + tuple = SearchSysCache1(AUTHNAME, PointerGetDatum(stmt->role));
> + if (!HeapTupleIsValid(tuple))
> + ereport(ERROR,
> + (errcode(ERRCODE_UNDEFINED_OBJECT),
> + errmsg("role \"%s\" does not exist", stmt->role)));
>
> To:
>
> if (strcmp(stmt->role, "current_user") == 0)
> roleid = GetUserId();
> else
> roleid = get_role_oid(stmt->role, false);
>
> tuple = SearchSysCache1(AUTHOID, ObjectIdGetDatum(roleid));
>
> if (!HeapTupleIsValid(tuple))
> ereport(ERROR,
> (errcode(ERRCODE_UNDEFINED_OBJECT),
> errmsg("roleid %d does not exist", roleid)));
>
> I think this makes it a bit cleaner. It also reuses existing code as
> 'get_role_oid()' already does a valid role name check and will raise the
> proper error.
Year, far cleaner. I missed the function. Thank you.
regards,
--
Kyotaro Horiguchi
NTT Open Source Software Center
