Adam,
* Adam Brightwell (adam.brightwell@crunchydatasolutions.com) wrote:
> Given this discussion, I have attached a patch that removes CATUPDATE for
> review/discussion.
Thanks!
> One of the interesting behaviors (or perhaps not) is how 'pg_class_aclmask'
> handles an invalid role id when checking permissions against 'rolsuper'
> instead of 'rolcatupdate'. This is demonstrated by the
> 'has_table_privilege' regression test expected results. In summary,
> 'has_rolcatupdate' raises an error when an invalid OID is provided,
> however, as documented in the source 'superuser_arg' does not, simply
> returning false. Therefore, when checking for superuser-ness of an
> non-existent role, the result will be false and not an error. Perhaps this
> is OK, but my concern would be on the expected behavior around items like
> 'has_table_privilege'. My natural thought would be that we would want to
> preserve that specific functionality, though short of adding a
> 'has_rolsuper' function that will raise an appropriate error, I'm uncertain
> of an approach. Thoughts?
This role oid check is only getting called in this case because it's
pg_authid which is getting checked (because it's a system catalog table)
and it's then falling into this one case. I don't think we need to
preserve that.
If you do the same query with that bogus OID but a normal table, you get
the same 'f' result that the regression test gives with this change.
We're not back-patching this and I seriously doubt anyone is relying on
this special response for system catalog tables.
So, unless anyone wants to object, I'll continue to look over this patch
for eventual application against master. If there are objections, it'd
be great if they could be voiced sooner rather than later.
Thanks!
Stephen