Re: How about to have relnamespace and relrole? - Mailing list pgsql-hackers

<div dir="ltr">Hi,<br /><br />Personally, I was looking for something like this as I need to see rolename<br />and
namespacename many times in my queries rather than it's oid.<br />But making a JOIN expression every-time was a pain.
Thiscertainly makes it<br />easier. And I see most DBAs are looking for it.<br /><br />I agree on Tom's concern on MVCC
issue,but we already hit that when we<br />introduced regclass and others. So I see no additional issue with these<br
/>changesas such. About planner slowness, I guess updated documentation looks<br />perfect for that.<br /><br />So I
wentahead reviewing these patches.<br /><br />All patches are straight forward and since we have similar code
already<br/>exists, I did not find any issue at code level. They are consistent with other<br />functions.<br /><br
/>Patchesapplies with patch -p1. make, make install, make check has<br />no issues. Testing was fine too.<br /><br
/>Howeverhere are few review comments on the patches attached:<br /><br />Review points on 0001-Add-regrole.patch<br
/>---<br/>1.<br /><span style="font-family:monospace,monospace">+#include "utils/acl.h" <br /></span><br />Can you
pleaseadd it at appropriate place as #include list is an ordered list<br /><br />2.<br /><span
style="font-family:monospace,monospace">+   char       *role_or_oid = PG_GETARG_CSTRING(0);<br /></span><br />Can we
havevariable named as role_name_or_oid, like other similar functions?<br /><br />3.<br /><span
style="font-family:monospace,monospace">+   /* <br />+     * Normal case: parse the name into components and see if it
matchesany <br />+     * pg_role entries in the current search path. <br />+     */<br /></span><br />I believe, roles
arenever searched per search path. Thus need to update<br />comments here.<br /><br /><br />Review points on
0002-Add-regnamespace.patch<br/>---<br />1.<br /><span style="font-family:monospace,monospace">+ * regnamespacein   
   - converts "classname" to class OID <br /></span><br />Typos. Should be nspname instead of classname and namespase
OIDinstead of<br />class OID<br /><br /><br />Review points on 0003-Check-new....patch<br />---<br />1.<br /><span
style="font-family:monospace,monospace">@@-1860,6 +1860,7 @@ StoreAttrDefault(Relation rel, AttrNumber attnum, <br
/>    Oid            attrdefOid; <br />     ObjectAddress colobject, <br />                 defobject; <br />+   
Oid           exprtype; <br /></span><br />This seems unrelated. Please remove.<br /><br /><br />Apart from this, it
willbe good if you have ONLY two patches,<br />(1) For regrole and (2) For regnamespace specific<br />With all related
changesinto one patch. I mean, all role related changes i.e.<br />0001 + 0003 role related changes + 0004 role related
changesand docs update<br />AND<br />0002 + 0003 nsp related changes + 0004 nsp related changes<br /><br />Thanks<br
/><br/><div class="gmail_extra">-- <br /><div class="gmail_signature"><div dir="ltr">Jeevan B Chalke<br />Principal
SoftwareEngineer, Product Development<br />EnterpriseDB Corporation<br />The Enterprise PostgreSQL Company<br /><br
/></div></div></div></div>