Re: BUG #13666: REASSIGN OWNED BY doesn't affect the relation underlying composite type

From: Alvaro Herrera <alvherre(at)2ndquadrant(dot)com>
To: cpacejo(at)clearskydata(dot)com
Cc: pgsql-bugs(at)postgresql(dot)org, Robert Haas <robertmhaas(at)gmail(dot)com>, Kouhei Kaigai <kaigai(at)ak(dot)jp(dot)nec(dot)com>
Subject: Re: BUG #13666: REASSIGN OWNED BY doesn't affect the relation underlying composite type
Date: 2015-12-15 23:46:51
Message-ID: 20151215234651.GF2618@alvherre.pgsql
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-bugs

Alvaro Herrera wrote:

> I think the bug is that shdepReassignOwned shouldn't be calling
> AlterTypeOwnerInternal directly, because that routine is not of a high
> enough level. We need another routine sitting between AlterTypeOwner
> and AlterTypeOwnerInternal which does the real work for the former
> (including calling ATExecChangeOwner for composites), after looking up
> the type OID from the name list; then shdepReassignOwned should call
> that new intermediate function.

I think that code is a bit confused. In the attached patch I renamed
AlterTypeOwnerInternal to AlterTypeOwnerBare, and created a new
AlterTypeOwnerInternal which either calls ATExecChangeOwner (if
composite) of AlterTypeOwnerBare (if not); then, on ATExecChangeOwner we
can call *Bare if necessary, and on REASSIGN OWNED (shdepReassignOwned)
we can call AlterTypeOwnerInternal which now behaves correctly in the
case of a composite type. There's some duplicate code that was
previously part of AlterTypeOwner directly; that's now calling
AlterTypeOwnerInternal instead.

There's an imcomplete comment above AlterTypeOwnerInternal related to a
function parameter that doesn't exist, introduced by 05f3f9c7b292; I
think this was supposed to control whether object access hooks are
called or not. I wonder if we need to fix that -- this patch simply
removes the comment. Cc'ing KaiGai and Robert about that.

(There's some inefficiency now that wasn't previously in that we open
pg_type repeatedly in ALTER TYPE OWNER, but I don't think we care about
that.)

This has been wrong all along; I wonder how come we hadn't gotten any
reports about this. That said, the test coverage for REASSIGN OWNED
leaves something to be desired. This patch adds a composite type to
that, but there's a lot more object types that would be worth covering.
(Note the supplied test case uses a "regrole" cast, so it's not
appropriate to apply that to 9.4 and earlier.)

--
Álvaro Herrera http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

Attachment Content-Type Size
reassign-composites.patch text/x-diff 9.2 KB

In response to

Responses

Browse pgsql-bugs by date

  From Date Subject
Next Message Yang, Leo 2015-12-16 07:34:33 server closed the connection unexpectedly during upgrade
Previous Message Joe Conway 2015-12-15 23:17:25 Re: json_to_recordset not working with camelcase json keys