Thread: Extra quote_all_identifiers in _dumpOptions

Extra quote_all_identifiers in _dumpOptions

From
Arthur Zakirov
Date:
Hello hackers,

While working on pg_dump I noticed the extra quote_all_identifiers in 
_dumpOptions struct. I attached the patch.

It came from refactoring by 0eea8047bf. There is also a discussion:
https://www.postgresql.org/message-id/CACw0%2B13ZUcXbj9GKJNGZTkym1SXhwRu28nxHoJMoX5Qwmbg_%2Bw%40mail.gmail.com

Initially the patch proposed to use quote_all_identifiers of 
_dumpOptions. But then everyone came to a decision to use global 
quote_all_identifiers from string_utils.c, because fmtId() uses it.

-- 
Arthur Zakirov
Postgres Professional: http://www.postgrespro.com
Russian Postgres Company

Attachment

Re: Extra quote_all_identifiers in _dumpOptions

From
Bruce Momjian
Date:
On Thu, Jun 27, 2019 at 12:12:15PM +0300, Arthur Zakirov wrote:
> Hello hackers,
> 
> While working on pg_dump I noticed the extra quote_all_identifiers in
> _dumpOptions struct. I attached the patch.
> 
> It came from refactoring by 0eea8047bf. There is also a discussion:
> https://www.postgresql.org/message-id/CACw0%2B13ZUcXbj9GKJNGZTkym1SXhwRu28nxHoJMoX5Qwmbg_%2Bw%40mail.gmail.com
> 
> Initially the patch proposed to use quote_all_identifiers of _dumpOptions.
> But then everyone came to a decision to use global quote_all_identifiers
> from string_utils.c, because fmtId() uses it.
> 
> -- 
> Arthur Zakirov
> Postgres Professional: http://www.postgrespro.com
> Russian Postgres Company

> diff --git a/src/bin/pg_dump/pg_backup.h b/src/bin/pg_dump/pg_backup.h
> index db30b54a92..8c0cedcd98 100644
> --- a/src/bin/pg_dump/pg_backup.h
> +++ b/src/bin/pg_dump/pg_backup.h
> @@ -153,7 +153,6 @@ typedef struct _dumpOptions
>      int            no_synchronized_snapshots;
>      int            no_unlogged_table_data;
>      int            serializable_deferrable;
> -    int            quote_all_identifiers;
>      int            disable_triggers;
>      int            outputNoTablespaces;
>      int            use_setsessauth;

Wow, good catch.  I thought C compilers would have reported this issue,
but obviously not.  Patch applied to head.  Thanks.

-- 
  Bruce Momjian  <bruce@momjian.us>        http://momjian.us
  EnterpriseDB                             http://enterprisedb.com

+ As you are, so once was I.  As I am, so you will be. +
+                      Ancient Roman grave inscription +



Re: Extra quote_all_identifiers in _dumpOptions

From
Michael Paquier
Date:
On Mon, Jul 08, 2019 at 07:32:07PM -0400, Bruce Momjian wrote:
> Wow, good catch.  I thought C compilers would have reported this issue,
> but obviously not.  Patch applied to head.  Thanks.

Yes, I don't recall that gcc nor clang have a magic recipy for that.
We have a couple of other orphaned ones in the backend actually.
--
Michael

Attachment

Re: Extra quote_all_identifiers in _dumpOptions

From
Arthur Zakirov
Date:
On 09.07.2019 02:32, Bruce Momjian wrote:
>> diff --git a/src/bin/pg_dump/pg_backup.h b/src/bin/pg_dump/pg_backup.h
>> index db30b54a92..8c0cedcd98 100644
>> --- a/src/bin/pg_dump/pg_backup.h
>> +++ b/src/bin/pg_dump/pg_backup.h
>> @@ -153,7 +153,6 @@ typedef struct _dumpOptions
>>       int            no_synchronized_snapshots;
>>       int            no_unlogged_table_data;
>>       int            serializable_deferrable;
>> -    int            quote_all_identifiers;
>>       int            disable_triggers;
>>       int            outputNoTablespaces;
>>       int            use_setsessauth;
> 
> Wow, good catch.  I thought C compilers would have reported this issue,
> but obviously not.  Patch applied to head.  Thanks.

Thank you, Bruce!

-- 
Arthur Zakirov
Postgres Professional: http://www.postgrespro.com
Russian Postgres Company