On 2019-Jun-10, Daniel Gustafsson wrote:
> +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,
-1 on this approach.  Having this ownercheck function return the owning
object ID seems way too strange.  I'd rather not have the new ownercheck
function, and instead do a syscache search to obtain the type OID in
check_object_ownership, then do pg_type_ownercheck.  I'm not even sure
that pg_domain_constraint_ownercheck makes a lot of sense in itself,
since it's never the constraint that requires an owner check.
-- 
Álvaro Herrera                https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services