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