Re: BUG #13666: REASSIGN OWNED BY doesn't affect the relation underlying composite type
Alvaro Herrera <alvherre@2ndquadrant.com>
From: Alvaro Herrera <alvherre@2ndquadrant.com>
To: cpacejo@clearskydata.com
Cc: pgsql-bugs@postgresql.org, Robert Haas <robertmhaas@gmail.com>,
Kouhei Kaigai <kaigai@ak.jp.nec.com>
Date: 2015-12-15T23:46:51Z
Lists: pgsql-bugs
Attachments
- reassign-composites.patch (text/x-diff) patch
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
Commits
-
adjust ACL owners for REASSIGN and ALTER OWNER TO
- ab14c1383836 9.1.20 landed
- 7af3dd540e29 9.2.15 landed
- 62e6eba8d266 9.3.11 landed
- 2c8ae6442fed 9.4.6 landed
- 59367fdf97cc 9.5.0 cited