Date: Mon, 10 Jun 2019 08:55:27 -0400
From: Alvaro Herrera <alvherre@2ndquadrant.com>
To: Daniel Gustafsson <daniel@yesql.se>
Cc: Michael Paquier <michael@paquier.xyz>, clemens@ladisch.de,
	PostgreSQL mailing lists <pgsql-bugs@lists.postgresql.org>
Subject: Re: BUG #15833: defining a comment on a domain constraint fails with
 wrong OID
Message-ID: <20190610125527.GA30133@alvherre.pgsql>
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <03D5C161-2624-4131-9BFF-16BF1D0CA1F2@yesql.se>
User-Agent: Mutt/1.9.4 (2018-02-28)
Precedence: bulk

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