> On 10 Jun 2019, at 08:28, Michael Paquier <michael@paquier.xyz> wrote:
>
> On Fri, Jun 07, 2019 at 02:42:33PM +0900, Michael Paquier wrote:
>> Well, it wouldn't be a problem to do a syscache lookup and then use
>> the type from contypid, no? It seems to me that it would be more
>> consistent to just add a pg_domain_constraint_ownercheck() in aclchk.c
>> as all the syscache lookups happen their for all the other objects
>> types. What do you think?
>
> Attached is what I have in mind. There are already tests at the
> bottom of constraints.source checking for comments on both table and
> domain constraints, so my proposal is to run them with a dedicated
> role. What do you think?
+1 on the approach of the patch, it seems like the simplest approach. A
comment on the check_object_ownership() diff though:
+ if (!pg_domain_constraint_ownercheck(address.objectId, roleid))
+ aclcheck_error_type(ACLCHECK_NOT_OWNER, address.objectId);
This doesn’t work for the errorcase as the address.objectId is the wrong Oid
here as well, the contypid extracted in pg_domain_constraint_ownercheck() is
required. I’ve hacked up your patch to pass it back and that seems to work,
and also added a test for the errorpath. Another option would be to provide a
new aclcheck_error_domain_constraint(), not sure which is best.
cheers ./daniel
