Re: Grantor name gets lost when grantor role dropped

Alvaro Herrera wrote:
> Russell Smith wrote:
>
>> Alvaro Herrera wrote:
>>
>>> Jeff Davis wrote:
>>>
>>>
>>>
>>>> CREATE ROLE test_role
>>>> NOSUPERUSER INHERIT NOCREATEDB NOCREATEROLE;
>>>>
>>>> CREATE ROLE invalid_grantor
>>>> SUPERUSER INHERIT NOCREATEDB NOCREATEROLE;
>>>>
>>>> SET ROLE invalid_grantor;
>>>> GRANT "postgres" TO "test_role";
>>>> SET ROLE postgres;
>>>>
>>>> select * from pg_roles;
>>>>
>>>> select pg_auth_members.*, ur.rolname, gr.rolname from pg_auth_members
>>>> LEFT JOIN pg_roles ur ON roleid = oid
>>>> LEFT JOIN pg_roles gr ON gr.oid = grantor;
>>>>
>>>> DROP ROLE invalid_grantor;
>>>>
>>>> select pg_auth_members.*, ur.rolname, gr.rolname from pg_auth_members
>>>> LEFT JOIN pg_roles ur ON roleid = oid
>>>> LEFT JOIN pg_roles gr ON gr.oid = grantor;
>>>>
>>>> DROP ROLE test_role;
>>>>
>>>>
>>> The problem here is that we allowed the drop of invalid_grantor. We are
>>> missing a shared dependency on it.
>>>
>>>
>> So does this make a todo item?
>>
>> But this still leaves the concerns about you can currently get the
>> database into an invalid state that can't be dumped and restored.
>>
>
> Correct, which makes it a bug (==> needs fixed) rather than a todo item.
>
> We now have a problem because there may already be databases that are
> undumpable. We might need to provide a workaround for people with such
> a database.
>
Well, given GRANTED BY is not documented, it should be reasonable to
alter pg_dumpall to remove GRANTED BY in cases where the role doesn't
resolve.

That is not an unsafe change as it should never happen if the depend
data is in place. It will also allow any currently undumpable databases
to be dumpable again.

A simple and totally untested or compiled patch is below;

Index: pg_dumpall.c
===================================================================
RCS file: /projects/cvsroot/pgsql/src/bin/pg_dump/pg_dumpall.c,v
retrieving revision 1.90
diff -c -r1.90 pg_dumpall.c
*** pg_dumpall.c 10 Feb 2007 14:58:55 -0000 1.90
--- pg_dumpall.c 18 Apr 2007 07:41:44 -0000
***************
*** 724,730 ****
fprintf(OPF, " TO %s", fmtId(member));
if (*option == 't')
fprintf(OPF, " WITH ADMIN OPTION");
! fprintf(OPF, " GRANTED BY %s;\n", fmtId(grantor));
}

PQclear(res);
--- 724,739 ----
fprintf(OPF, " TO %s", fmtId(member));
if (*option == 't')
fprintf(OPF, " WITH ADMIN OPTION");
! /*
! * It's possible due to a lack of shared dependency tracking
! * of grantor that this parameter and be an empty string.
! * In that case, we don't dump the grantor and the grant
! * will be granted by the user who imports the roles.
! */
! if (strlen(grantor) != 0)
! fprintf(OPF, " GRANTED BY %s", fmtId(grantor));
!
! fprintf(OPF, ";\n");
}

PQclear(res);