Re: patch: move dumpUserConfig call in dumpRoles function of pg_dumpall.c

On Mon, Oct 10, 2011 at 11:54 AM, Robert Haas <robertmhaas(at)gmail(dot)com> wrote:
> On Thu, Aug 4, 2011 at 2:04 PM, Robert Haas <robertmhaas(at)gmail(dot)com> wrote:
>> On Thu, Aug 4, 2011 at 1:53 PM, Phil Sorber <phil(at)omniti(dot)com> wrote:
>>> Ok, here is the patch that just moves the ALTER/SET pieces to the end.
>>> Can we get this included in the next commit fest?
>>
>> Yep, just make yourself an account and add it.
>
> Unfortunately, it doesn't look like anyone ever replied to this
> thread, but Tom posted some thoughts on another thread that seem to me
> to be a serious problem for this patch:
>
> http://archives.postgresql.org/message-id/13764.1315094292@sss.pgh.pa.us
>
> I don't see any easy way around that problem, so I'm going to mark
> this patch Returned with Feedback for now.  It strikes me as craziness
> to try to guess which settings we should restore at the beginning and
> which at the end, so I think we need a better idea.  I don't really
> understand why it's not OK to just have pg_dump issue RESET ROLE at
> appropriate points in the process; that seems like it would be
> sufficient and not particularly ugly.
>
> --
> Robert Haas
> EnterpriseDB: http://www.enterprisedb.com
> The Enterprise PostgreSQL Company
>

I am going to remove that patch from the commit fest because we all
agree that it does not solve the problem satisfactorily. I would like
to re-iterate a few points, and submit a new patch.

First off, there are two distinct problems here that we have been
lumping into one. There is the issue of the 'ALTER DATABASE SET ROLE'
and then there is the 'ALTER ROLE SET ROLE' case. The former is the
one that has been causing us so many problems, and the latter is the
one that I really care about.

Also, there are more people that are hitting this issue as well:

http://archives.postgresql.org/pgsql-hackers/2011-02/msg02362.php

My proposal would be to table the discussion about the 'ALTER DATABASE
SET ROLE' case because there seems to be a bit of a philosophical
debate behind this that needs to be sorted out first.

If we focus only on the 'ALTER ROLE' case I think there is a trivial
solution. We already separate the CREATE from the ALTER in a single
loop. We also already separate out the user config in a separate
function called from this same loop. The problem is that the user
config can be dependent upon a later CREATE. So all I suggest is to
move the user config dumping into a new loop afterward so that the
user config ALTER's come after all the other CREATE's and ALTER's. It
amounts to a 7 line change and solves our problem rather elegantly.