Re: BUG #15833: defining a comment on a domain constraint fails with wrong OID

> 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