Extra quote_all_identifiers in _dumpOptions

Started by Arthur Zakirovover 6 years ago4 messages
#1Arthur Zakirov
a.zakirov@postgrespro.ru
1 attachment(s)

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:
/messages/by-id/CACw0+13ZUcXbj9GKJNGZTkym1SXhwRu28nxHoJMoX5Qwmbg_+w@mail.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

Attachments:

extra_quote_all_identifiers.patchtext/x-patch; name=extra_quote_all_identifiers.patchDownload
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;
#2Bruce Momjian
bruce@momjian.us
In reply to: Arthur Zakirov (#1)
Re: Extra quote_all_identifiers in _dumpOptions

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:
/messages/by-id/CACw0+13ZUcXbj9GKJNGZTkym1SXhwRu28nxHoJMoX5Qwmbg_+w@mail.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 +
#3Michael Paquier
michael@paquier.xyz
In reply to: Bruce Momjian (#2)
Re: Extra quote_all_identifiers in _dumpOptions

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

#4Arthur Zakirov
a.zakirov@postgrespro.ru
In reply to: Bruce Momjian (#2)
Re: Extra quote_all_identifiers in _dumpOptions

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