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