Re: pg_dump roles support

Hi,

Thank you for your review.

On 2008-11-07 21:20, Alvaro Herrera wrote:
>> The patch contains the following things:
>>
>>
>> - pg_dump and pg_dumpall accepts the --role=rolename parameter, and
>> sends a SET ROLE command on their connections
>>
>
> Minor comment -- I think you need to quote the role name in the SET
> command. Otherwise roles with funny names will fail (try a role with a
> space for example)
>
>
Of course you need to quote the role names with special characters in it.
I tested it this way (from bash):
$ src/bin/pg_dump/pg_dump -h localhost -p 4003 --role "asd ' \" qwe" test
Note the bash style escaping of the string [asd ' " qwe].
It created a dump file with SET role = "asd ' "" qwe"; line in it. Seems
fine for me.
> The SGML patch seems to contain unnecessary whitespace changes; please
> clean that up.
>
Maybe you missed an updated version of the patch? Available here:
http://archives.postgresql.org/pgsql-hackers/2008-11/msg00391.php

>> + /* te->defn should have the form SET role = 'foo'; */
>> + char *defn = strdup(te->defn);
>> + char *ptr1;
>> + char *ptr2 = NULL;
>> +
>> + ptr1 = strchr(defn, '\'');
>> + if (ptr1)
>> + ptr2 = strchr(++ptr1, '\'');
>>
>
> Does this work if the role name contains a ' ?
>
Right, this one fails with ' in the role name. An update coming soon closing this issue.

Regards,

Benedek Laszlo