Thread: Ticket 266: Unable to backup table when quotation marks used in the name

Ticket 266: Unable to backup table when quotation marks used in the name

From
Guillaume Lelarge
Date:
Hi,

I worked on this bug since sunday evening, and it's really an annoying
one. We have an issue with objects' names containing double quotes when
doing a backup, whatever your OS is.

I found a fix, and I would usually commit it ASAP. But this time, I'm a
bit worried. I don't understand why the commandLineCleanOption()
function (in misc.c) is so complicated. There should be a reason.
Unfortunately, The fix I wrote deletes almost all of it.

If someone has an explanation of the function, I'm happy to hear it.

And if someone thinks my patch is just plain stupid, tell me before I
commit it :)

Thanks.


--
Guillaume
 http://www.postgresql.fr
 http://dalibo.com

Attachment

Re: Ticket 266: Unable to backup table when quotation marks used in the name

From
Ashesh Vashi
Date:
Hi Guillaume,

As long as I do remember, the commandLineClenOption() function was introduced, because of the command-line parameter behaves differently with quote on different platforms.

Though - I have tested your patch (Sorry - I do not have enough time to spend around pgAdmin now-a-days because of the work load).
But It will be good - if you can please test the patch on all the platform (specially on windows) with quote in the name for table, user-name, etc before applying this patch.

--
Thanks & Regards,


Ashesh Vashi
EnterpriseDB INDIA:
Enterprise Postgres Company

 

http://www.linkedin.com/in/asheshvashi



On Thu, Nov 18, 2010 at 4:01 AM, Guillaume Lelarge <guillaume@lelarge.info> wrote:
Hi,

I worked on this bug since sunday evening, and it's really an annoying
one. We have an issue with objects' names containing double quotes when
doing a backup, whatever your OS is.

I found a fix, and I would usually commit it ASAP. But this time, I'm a
bit worried. I don't understand why the commandLineCleanOption()
function (in misc.c) is so complicated. There should be a reason.
Unfortunately, The fix I wrote deletes almost all of it.

If someone has an explanation of the function, I'm happy to hear it.

And if someone thinks my patch is just plain stupid, tell me before I
commit it :)

Thanks.


--
Guillaume
 http://www.postgresql.fr
 http://dalibo.com


--
Sent via pgadmin-hackers mailing list (pgadmin-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgadmin-hackers


Re: Ticket 266: Unable to backup table when quotation marks used in the name

From
Guillaume Lelarge
Date:
Le 18/11/2010 05:25, Ashesh Vashi a écrit :
> Hi Guillaume,
>
> As long as I do remember, the commandLineClenOption() function was
> introduced, because of the command-line parameter behaves differently with
> quote on different platforms.
>

I figured that but there's no OS specific code.

> Though - I have tested your patch (Sorry - I do not have enough time to
> spend around pgAdmin now-a-days because of the work load).
> But It will be good - if you can please test the patch on all the platform
> (specially on windows) with quote in the name for table, user-name, etc
> before applying this patch.

I did quite a lot of tests:

 * on Linux

ticket266=# \d
         List of relations
 Schema | Name | Type  |   Owner
--------+------+-------+-----------
 S"1    | TN5  | table | guillaume
 S"1    | tn2  | table | guillaume
 public | T1   | table | guillaume
 public | t"3  | table | guillaume
 public | t2   | table | guillaume
(5 rows)

 * on Windows, much more but I don't have the list here (I created
   tables like t1, t"1, t "1, t" 1, t " 1, and the same for schemas)

I'll try to have a set of tests to try on the different platforms.
Unfortunately, I still can't test on MacOSX.

Do you have any specific interesting tests?

Thanks.


--
Guillaume
 http://www.postgresql.fr
 http://dalibo.com

Re: Ticket 266: Unable to backup table when quotation marks used in the name

From
Guillaume Lelarge
Date:
Le 18/11/2010 09:08, Guillaume Lelarge a écrit :
> Le 18/11/2010 05:25, Ashesh Vashi a écrit :
>> Hi Guillaume,
>>
>> As long as I do remember, the commandLineClenOption() function was
>> introduced, because of the command-line parameter behaves differently with
>> quote on different platforms.
>>
>
> I figured that but there's no OS specific code.
>
>> Though - I have tested your patch (Sorry - I do not have enough time to
>> spend around pgAdmin now-a-days because of the work load).
>> But It will be good - if you can please test the patch on all the platform
>> (specially on windows) with quote in the name for table, user-name, etc
>> before applying this patch.
>
> I did quite a lot of tests:
>
>  * on Linux
>
> ticket266=# \d
>          List of relations
>  Schema | Name | Type  |   Owner
> --------+------+-------+-----------
>  S"1    | TN5  | table | guillaume
>  S"1    | tn2  | table | guillaume
>  public | T1   | table | guillaume
>  public | t"3  | table | guillaume
>  public | t2   | table | guillaume
> (5 rows)
>
>  * on Windows, much more but I don't have the list here (I created
>    tables like t1, t"1, t "1, t" 1, t " 1, and the same for schemas)
>
> I'll try to have a set of tests to try on the different platforms.
> Unfortunately, I still can't test on MacOSX.
>

I checked on Linux and Windows, and it works great on both. If someone
can try my patch on Mac OS X, you'll find attached a custom (-Fc) backup
of a small database with every kind of strangely named objects.

> Do you have any specific interesting tests?
>

And if you have that, I'm really interested.


--
Guillaume
 http://www.postgresql.fr
 http://dalibo.com

Attachment

Re: Ticket 266: Unable to backup table when quotation marks used in the name

From
Dave Page
Date:
On Thu, Nov 18, 2010 at 11:36 PM, Guillaume Lelarge
<guillaume@lelarge.info> wrote:
> Le 18/11/2010 09:08, Guillaume Lelarge a écrit :
>> Le 18/11/2010 05:25, Ashesh Vashi a écrit :
>>> Hi Guillaume,
>>>
>>> As long as I do remember, the commandLineClenOption() function was
>>> introduced, because of the command-line parameter behaves differently with
>>> quote on different platforms.
>>>
>>
>> I figured that but there's no OS specific code.
>>
>>> Though - I have tested your patch (Sorry - I do not have enough time to
>>> spend around pgAdmin now-a-days because of the work load).
>>> But It will be good - if you can please test the patch on all the platform
>>> (specially on windows) with quote in the name for table, user-name, etc
>>> before applying this patch.
>>
>> I did quite a lot of tests:
>>
>>  * on Linux
>>
>> ticket266=# \d
>>          List of relations
>>  Schema | Name | Type  |   Owner
>> --------+------+-------+-----------
>>  S"1    | TN5  | table | guillaume
>>  S"1    | tn2  | table | guillaume
>>  public | T1   | table | guillaume
>>  public | t"3  | table | guillaume
>>  public | t2   | table | guillaume
>> (5 rows)
>>
>>  * on Windows, much more but I don't have the list here (I created
>>    tables like t1, t"1, t "1, t" 1, t " 1, and the same for schemas)
>>
>> I'll try to have a set of tests to try on the different platforms.
>> Unfortunately, I still can't test on MacOSX.
>>
>
> I checked on Linux and Windows, and it works great on both. If someone
> can try my patch on Mac OS X, you'll find attached a custom (-Fc) backup
> of a small database with every kind of strangely named objects.

I tried backing up all the oddly named tables on OSX, and didn't get any errors.


--
Dave Page
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake

EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

Re: Ticket 266: Unable to backup table when quotation marks used in the name

From
Guillaume Lelarge
Date:
Le 19/11/2010 14:47, Dave Page a écrit :
> On Thu, Nov 18, 2010 at 11:36 PM, Guillaume Lelarge
> <guillaume@lelarge.info> wrote:
>> Le 18/11/2010 09:08, Guillaume Lelarge a écrit :
>>> Le 18/11/2010 05:25, Ashesh Vashi a écrit :
>>>> Hi Guillaume,
>>>>
>>>> As long as I do remember, the commandLineClenOption() function was
>>>> introduced, because of the command-line parameter behaves differently with
>>>> quote on different platforms.
>>>>
>>>
>>> I figured that but there's no OS specific code.
>>>
>>>> Though - I have tested your patch (Sorry - I do not have enough time to
>>>> spend around pgAdmin now-a-days because of the work load).
>>>> But It will be good - if you can please test the patch on all the platform
>>>> (specially on windows) with quote in the name for table, user-name, etc
>>>> before applying this patch.
>>>
>>> I did quite a lot of tests:
>>>
>>>  * on Linux
>>>
>>> ticket266=# \d
>>>          List of relations
>>>  Schema | Name | Type  |   Owner
>>> --------+------+-------+-----------
>>>  S"1    | TN5  | table | guillaume
>>>  S"1    | tn2  | table | guillaume
>>>  public | T1   | table | guillaume
>>>  public | t"3  | table | guillaume
>>>  public | t2   | table | guillaume
>>> (5 rows)
>>>
>>>  * on Windows, much more but I don't have the list here (I created
>>>    tables like t1, t"1, t "1, t" 1, t " 1, and the same for schemas)
>>>
>>> I'll try to have a set of tests to try on the different platforms.
>>> Unfortunately, I still can't test on MacOSX.
>>>
>>
>> I checked on Linux and Windows, and it works great on both. If someone
>> can try my patch on Mac OS X, you'll find attached a custom (-Fc) backup
>> of a small database with every kind of strangely named objects.
>
> I tried backing up all the oddly named tables on OSX, and didn't get any errors.
>

So it works on each supported platform that I know of. Unless there is
tests I forgot to do, I'm inclined to commit this. Any last minute
objections?


--
Guillaume
 http://www.postgresql.fr
 http://dalibo.com

Re: Ticket 266: Unable to backup table when quotation marks used in the name

From
Guillaume Lelarge
Date:
Le 20/11/2010 12:13, Guillaume Lelarge a écrit :
> Le 19/11/2010 14:47, Dave Page a écrit :
>> On Thu, Nov 18, 2010 at 11:36 PM, Guillaume Lelarge
>> <guillaume@lelarge.info> wrote:
>>> Le 18/11/2010 09:08, Guillaume Lelarge a écrit :
>>>> Le 18/11/2010 05:25, Ashesh Vashi a écrit :
>>>>> Hi Guillaume,
>>>>>
>>>>> As long as I do remember, the commandLineClenOption() function was
>>>>> introduced, because of the command-line parameter behaves differently with
>>>>> quote on different platforms.
>>>>>
>>>>
>>>> I figured that but there's no OS specific code.
>>>>
>>>>> Though - I have tested your patch (Sorry - I do not have enough time to
>>>>> spend around pgAdmin now-a-days because of the work load).
>>>>> But It will be good - if you can please test the patch on all the platform
>>>>> (specially on windows) with quote in the name for table, user-name, etc
>>>>> before applying this patch.
>>>>
>>>> I did quite a lot of tests:
>>>>
>>>>  * on Linux
>>>>
>>>> ticket266=# \d
>>>>          List of relations
>>>>  Schema | Name | Type  |   Owner
>>>> --------+------+-------+-----------
>>>>  S"1    | TN5  | table | guillaume
>>>>  S"1    | tn2  | table | guillaume
>>>>  public | T1   | table | guillaume
>>>>  public | t"3  | table | guillaume
>>>>  public | t2   | table | guillaume
>>>> (5 rows)
>>>>
>>>>  * on Windows, much more but I don't have the list here (I created
>>>>    tables like t1, t"1, t "1, t" 1, t " 1, and the same for schemas)
>>>>
>>>> I'll try to have a set of tests to try on the different platforms.
>>>> Unfortunately, I still can't test on MacOSX.
>>>>
>>>
>>> I checked on Linux and Windows, and it works great on both. If someone
>>> can try my patch on Mac OS X, you'll find attached a custom (-Fc) backup
>>> of a small database with every kind of strangely named objects.
>>
>> I tried backing up all the oddly named tables on OSX, and didn't get any errors.
>>
>
> So it works on each supported platform that I know of. Unless there is
> tests I forgot to do, I'm inclined to commit this. Any last minute
> objections?
>

Commited.


--
Guillaume
 http://www.postgresql.fr
 http://dalibo.com

It looks like as feared, this patch did break the quoting of some
things, as I'm starting to get bugs reported internally here that are
not present in 1.10. Eg. this one, where a database name contains
quotes:

/opt/PostgresPlus/9.0AS/bin/pg_dump --host localhost --port 5444
--username "enterprisedb" --format custom --blobs --verbose --file
"/home/edb/abbas.backup" \"AB\"
pg_dump: [archiver (db)] connection to database ""AB"" failed: FATAL:
database ""AB"" does not exist
pg_dump: *** aborted because of error

Process returned exit code 1.

Can you check your work please Guillaume?

On Tue, Nov 23, 2010 at 11:13 AM, Guillaume Lelarge
<guillaume@lelarge.info> wrote:
> Le 20/11/2010 12:13, Guillaume Lelarge a écrit :
>> Le 19/11/2010 14:47, Dave Page a écrit :
>>> On Thu, Nov 18, 2010 at 11:36 PM, Guillaume Lelarge
>>> <guillaume@lelarge.info> wrote:
>>>> Le 18/11/2010 09:08, Guillaume Lelarge a écrit :
>>>>> Le 18/11/2010 05:25, Ashesh Vashi a écrit :
>>>>>> Hi Guillaume,
>>>>>>
>>>>>> As long as I do remember, the commandLineClenOption() function was
>>>>>> introduced, because of the command-line parameter behaves differently with
>>>>>> quote on different platforms.
>>>>>>
>>>>>
>>>>> I figured that but there's no OS specific code.
>>>>>
>>>>>> Though - I have tested your patch (Sorry - I do not have enough time to
>>>>>> spend around pgAdmin now-a-days because of the work load).
>>>>>> But It will be good - if you can please test the patch on all the platform
>>>>>> (specially on windows) with quote in the name for table, user-name, etc
>>>>>> before applying this patch.
>>>>>
>>>>> I did quite a lot of tests:
>>>>>
>>>>>  * on Linux
>>>>>
>>>>> ticket266=# \d
>>>>>          List of relations
>>>>>  Schema | Name | Type  |   Owner
>>>>> --------+------+-------+-----------
>>>>>  S"1    | TN5  | table | guillaume
>>>>>  S"1    | tn2  | table | guillaume
>>>>>  public | T1   | table | guillaume
>>>>>  public | t"3  | table | guillaume
>>>>>  public | t2   | table | guillaume
>>>>> (5 rows)
>>>>>
>>>>>  * on Windows, much more but I don't have the list here (I created
>>>>>    tables like t1, t"1, t "1, t" 1, t " 1, and the same for schemas)
>>>>>
>>>>> I'll try to have a set of tests to try on the different platforms.
>>>>> Unfortunately, I still can't test on MacOSX.
>>>>>
>>>>
>>>> I checked on Linux and Windows, and it works great on both. If someone
>>>> can try my patch on Mac OS X, you'll find attached a custom (-Fc) backup
>>>> of a small database with every kind of strangely named objects.
>>>
>>> I tried backing up all the oddly named tables on OSX, and didn't get any errors.
>>>
>>
>> So it works on each supported platform that I know of. Unless there is
>> tests I forgot to do, I'm inclined to commit this. Any last minute
>> objections?
>>
>
> Commited.
>
>
> --
> Guillaume
>  http://www.postgresql.fr
>  http://dalibo.com
>



--
Dave Page
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake

EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

Re: Ticket 266: Unable to backup table when quotation marks used in the name

From
Guillaume Lelarge
Date:
Le 06/01/2011 10:40, Dave Page a écrit :
> It looks like as feared, this patch did break the quoting of some
> things, as I'm starting to get bugs reported internally here that are
> not present in 1.10. Eg. this one, where a database name contains
> quotes:
>
> /opt/PostgresPlus/9.0AS/bin/pg_dump --host localhost --port 5444
> --username "enterprisedb" --format custom --blobs --verbose --file
> "/home/edb/abbas.backup" \"AB\"
> pg_dump: [archiver (db)] connection to database ""AB"" failed: FATAL:
> database ""AB"" does not exist
> pg_dump: *** aborted because of error
>
> Process returned exit code 1.
>
> Can you check your work please Guillaume?
>

I'm already working on it since Sean Ayres reported this bug (see "Error
with pgAdmin III backup/restore" thread on pgadmin-support).

I have a patch but I need to check it on Windows first.


--
Guillaume
 http://www.postgresql.fr
 http://dalibo.com

On Thu, Jan 6, 2011 at 2:02 PM, Guillaume Lelarge
<guillaume@lelarge.info> wrote:
> Le 06/01/2011 10:40, Dave Page a écrit :
>> It looks like as feared, this patch did break the quoting of some
>> things, as I'm starting to get bugs reported internally here that are
>> not present in 1.10. Eg. this one, where a database name contains
>> quotes:
>>
>> /opt/PostgresPlus/9.0AS/bin/pg_dump --host localhost --port 5444
>> --username "enterprisedb" --format custom --blobs --verbose --file
>> "/home/edb/abbas.backup" \"AB\"
>> pg_dump: [archiver (db)] connection to database ""AB"" failed: FATAL:
>> database ""AB"" does not exist
>> pg_dump: *** aborted because of error
>>
>> Process returned exit code 1.
>>
>> Can you check your work please Guillaume?
>>
>
> I'm already working on it since Sean Ayres reported this bug (see "Error
> with pgAdmin III backup/restore" thread on pgadmin-support).
>
> I have a patch but I need to check it on Windows first.

Cool, thanks.

--
Dave Page
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake

EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

Re: Ticket 266: Unable to backup table when quotation marks used in the name

From
Guillaume Lelarge
Date:
Le 06/01/2011 15:53, Dave Page a écrit :
> On Thu, Jan 6, 2011 at 2:02 PM, Guillaume Lelarge
> <guillaume@lelarge.info> wrote:
>> Le 06/01/2011 10:40, Dave Page a écrit :
>>> It looks like as feared, this patch did break the quoting of some
>>> things, as I'm starting to get bugs reported internally here that are
>>> not present in 1.10. Eg. this one, where a database name contains
>>> quotes:
>>>
>>> /opt/PostgresPlus/9.0AS/bin/pg_dump --host localhost --port 5444
>>> --username "enterprisedb" --format custom --blobs --verbose --file
>>> "/home/edb/abbas.backup" \"AB\"
>>> pg_dump: [archiver (db)] connection to database ""AB"" failed: FATAL:
>>> database ""AB"" does not exist
>>> pg_dump: *** aborted because of error
>>>
>>> Process returned exit code 1.
>>>
>>> Can you check your work please Guillaume?
>>>
>>
>> I'm already working on it since Sean Ayres reported this bug (see "Error
>> with pgAdmin III backup/restore" thread on pgadmin-support).
>>
>> I have a patch but I need to check it on Windows first.
>
> Cool, thanks.
>

Patch checked and pushed :)


--
Guillaume
 http://www.postgresql.fr
 http://dalibo.com