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

I have included two patches in this email. The first
(dump_user_config_last_with_set_role.patch) is an extension of my
first patch. In addition to moving the ALTER ROLE statements after the
CREATE ROLE statements it also inserts a SET ROLE after every connect.
It takes the role parameter from the --role command line option. This
fixes the problem of not being able to restore to a database because
of lack of permissions. This is similar to the idea proposed here:
http://archives.postgresql.org/pgsql-hackers/2010-12/msg01046.php

I think this patch is the better one for several reasons. It keeps the
ALTER ROLE and ALTER DATABASE statements in the same sections as their
related CREATE ROLE and CREATE DATABASE statements, thus not
dramatically altering the output of the command. It is a much simpler
non-invasive patch. It's concise and easy to understand. It solves all
the known scenario's that we have discussed previously.

That being said, I understand you have reservations about how it
works. You would prefer to move all of these type's of ALTER
statements to the end of the output. I also would prefer to see the
correct behavior included in the main tree over having my preference
over the changes. To that end I have also included a second patch
(dump_all_configs_last.patch) which handles it the way you prefer.
This creates new sections at the bottom for the ALTER statements.

These two patches are mutually exclusive. I would ask that you at
least consider the first patch before dismissing the idea. However, I
think applying either would would be an acceptable approach.

Thanks for your consideration.

On Thu, Jul 28, 2011 at 10:06 AM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> Robert Haas <robertmhaas(at)gmail(dot)com> writes:
>> On Wed, Jul 27, 2011 at 7:51 PM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
>>> I think pg_dumpall is the very least of your problems if you do
>>> something like that.  We probably ought to forbid it entirely.
>
>> Well, we had a long discussion of that on the thread Phil linked to,
>> and I don't think there was any consensus that forbidding it was the
>> right thing to do.
>
> You're right, I was half-remembering that thread and thinking that
> there are a lot of gotchas in doing an ALTER ROLE SET ROLE.  Florian
> claimed in the thread that he'd never hit one before, but that doesn't
> convince me much.
>
>> Phil appears to be trying to implement the
>> proposal you made here:
>> http://archives.postgresql.org/pgsql-hackers/2011-01/msg00452.php
>> ...although I don't think that what he did quite matches what you asked for.
>
> No, the proposed patch doesn't go nearly far enough to address Florian's
> problem.  What I was speculating about was moving all the role (and
> database) alters to the *end*, so they'd not take effect until after
> we'd completed all the restore actions.
>
>                        regards, tom lane
>