pg_dump(all) --quote-all-identifiers

Started by Robert Haasover 15 years ago6 messages
#1Robert Haas
robertmhaas@gmail.com
1 attachment(s)

In response to a complaint from Hartmut Goebel:

http://archives.postgresql.org/pgsql-bugs/2010-06/msg00018.php

And per a design proposed by Tom Lane:

http://archives.postgresql.org/pgsql-bugs/2010-06/msg00211.php

PFA a patch to implement $SUBJECT. One interesting aspect of this
patch is that types like "integer" and "double precision" don't get
quoted in the output, whereas types like "text" do. But it turns out
that types like "integer" and "double precision" don't *work* if
they're quoted, so this is not a bad thing. It might possibly be
judged to require documentation somewhere, however.

Suggestions welcome.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise Postgres Company

Attachments:

quote_all_identifiers.patchapplication/octet-stream; name=quote_all_identifiers.patchDownload
diff --git a/src/backend/utils/adt/ruleutils.c b/src/backend/utils/adt/ruleutils.c
index b85eefa..278e105 100644
--- a/src/backend/utils/adt/ruleutils.c
+++ b/src/backend/utils/adt/ruleutils.c
@@ -125,6 +125,7 @@ static SPIPlanPtr plan_getrulebyoid = NULL;
 static const char *query_getrulebyoid = "SELECT * FROM pg_catalog.pg_rewrite WHERE oid = $1";
 static SPIPlanPtr plan_getviewrule = NULL;
 static const char *query_getviewrule = "SELECT * FROM pg_catalog.pg_rewrite WHERE ev_class = $1 AND rulename = $2";
+bool quote_all_identifiers;
 
 
 /* ----------
@@ -6473,6 +6474,9 @@ quote_identifier(const char *ident)
 		}
 	}
 
+	if (quote_all_identifiers)
+		safe = false;
+
 	if (safe)
 	{
 		/*
diff --git a/src/backend/utils/misc/guc.c b/src/backend/utils/misc/guc.c
index 6597e18..bf16e03 100644
--- a/src/backend/utils/misc/guc.c
+++ b/src/backend/utils/misc/guc.c
@@ -1261,6 +1261,15 @@ static struct config_bool ConfigureNamesBool[] =
 		false, NULL, NULL
 	},
 
+	{
+		{"quote_all_identifiers", PGC_USERSET, COMPAT_OPTIONS,
+			gettext_noop("When generating SQL fragments, quote all identifiers."),
+			NULL,
+		},
+		&quote_all_identifiers,
+		false, NULL, NULL
+	},
+
 	/* End-of-list marker */
 	{
 		{NULL, 0, 0, NULL, NULL}, NULL, false, NULL, NULL
diff --git a/src/bin/pg_dump/dumputils.c b/src/bin/pg_dump/dumputils.c
index 4c5f9a5..3d26185 100644
--- a/src/bin/pg_dump/dumputils.c
+++ b/src/bin/pg_dump/dumputils.c
@@ -36,6 +36,8 @@ static bool parallel_init_done = false;
 static DWORD tls_index;
 #endif
 
+int quote_all_identifiers;
+
 void
 init_parallel_dump_utils(void)
 {
@@ -102,8 +104,10 @@ fmtId(const char *rawid)
 	 * These checks need to match the identifier production in scan.l. Don't
 	 * use islower() etc.
 	 */
+	if (quote_all_identifiers)
+		need_quotes = true;
 	/* slightly different rules for first character */
-	if (!((rawid[0] >= 'a' && rawid[0] <= 'z') || rawid[0] == '_'))
+	else if (!((rawid[0] >= 'a' && rawid[0] <= 'z') || rawid[0] == '_'))
 		need_quotes = true;
 	else
 	{
diff --git a/src/bin/pg_dump/dumputils.h b/src/bin/pg_dump/dumputils.h
index 6762839..77d05d0 100644
--- a/src/bin/pg_dump/dumputils.h
+++ b/src/bin/pg_dump/dumputils.h
@@ -46,4 +46,6 @@ extern bool processSQLNamePattern(PGconn *conn, PQExpBuffer buf,
 					  const char *schemavar, const char *namevar,
 					  const char *altnamevar, const char *visibilityrule);
 
+extern int quote_all_identifiers;
+
 #endif   /* DUMPUTILS_H */
diff --git a/src/bin/pg_dump/pg_dump.c b/src/bin/pg_dump/pg_dump.c
index 1c4867d..471a246 100644
--- a/src/bin/pg_dump/pg_dump.c
+++ b/src/bin/pg_dump/pg_dump.c
@@ -297,6 +297,7 @@ main(int argc, char **argv)
 		{"inserts", no_argument, &dump_inserts, 1},
 		{"lock-wait-timeout", required_argument, NULL, 2},
 		{"no-tablespaces", no_argument, &outputNoTablespaces, 1},
+		{"quote-all-identifiers", no_argument, &quote_all_identifiers, 1},
 		{"role", required_argument, NULL, 3},
 		{"use-set-session-authorization", no_argument, &use_setsessauth, 1},
 
@@ -635,6 +636,12 @@ main(int argc, char **argv)
 		do_sql_command(g_conn, "SET statement_timeout = 0");
 
 	/*
+	 * Quote all identifiers, if requested.
+	 */
+	if (quote_all_identifiers && g_fout->remoteVersion >= 90000)
+		do_sql_command(g_conn, "SET quote_all_identifiers = true");
+
+	/*
 	 * Start serializable transaction to dump consistent data.
 	 */
 	do_sql_command(g_conn, "BEGIN");
diff --git a/src/bin/pg_dump/pg_dumpall.c b/src/bin/pg_dump/pg_dumpall.c
index 0c3f63f..e0f0607 100644
--- a/src/bin/pg_dump/pg_dumpall.c
+++ b/src/bin/pg_dump/pg_dumpall.c
@@ -130,6 +130,7 @@ main(int argc, char *argv[])
 		{"inserts", no_argument, &inserts, 1},
 		{"lock-wait-timeout", required_argument, NULL, 2},
 		{"no-tablespaces", no_argument, &no_tablespaces, 1},
+		{"quote-all-identifiers", no_argument, &quote_all_identifiers, 1},
 		{"role", required_argument, NULL, 3},
 		{"use-set-session-authorization", no_argument, &use_setsessauth, 1},
 
@@ -328,6 +329,8 @@ main(int argc, char *argv[])
 		appendPQExpBuffer(pgdumpopts, " --inserts");
 	if (no_tablespaces)
 		appendPQExpBuffer(pgdumpopts, " --no-tablespaces");
+	if (quote_all_identifiers)
+		appendPQExpBuffer(pgdumpopts, " --quote-all-identifiers");
 	if (use_setsessauth)
 		appendPQExpBuffer(pgdumpopts, " --use-set-session-authorization");
 
@@ -440,6 +443,12 @@ main(int argc, char *argv[])
 		destroyPQExpBuffer(query);
 	}
 
+	/* Force quoting of all identifiers if requested. */
+	if (quote_all_identifiers && server_version >= 90000)
+	{
+		executeCommand(conn, "SET quote_all_identifiers = true");
+	}
+
 	fprintf(OPF, "--\n-- PostgreSQL database cluster dump\n--\n\n");
 	if (verbose)
 		dumpTimestamp("Started on");
diff --git a/src/include/utils/builtins.h b/src/include/utils/builtins.h
index ed64ebc..05bd5d8 100644
--- a/src/include/utils/builtins.h
+++ b/src/include/utils/builtins.h
@@ -583,6 +583,7 @@ extern Datum record_ge(PG_FUNCTION_ARGS);
 extern Datum btrecordcmp(PG_FUNCTION_ARGS);
 
 /* ruleutils.c */
+extern bool quote_all_identifiers;
 extern Datum pg_get_ruledef(PG_FUNCTION_ARGS);
 extern Datum pg_get_ruledef_ext(PG_FUNCTION_ARGS);
 extern Datum pg_get_viewdef(PG_FUNCTION_ARGS);
#2Bruce Momjian
bruce@momjian.us
In reply to: Robert Haas (#1)
Re: pg_dump(all) --quote-all-identifiers

Robert Haas wrote:

In response to a complaint from Hartmut Goebel:

http://archives.postgresql.org/pgsql-bugs/2010-06/msg00018.php

And per a design proposed by Tom Lane:

http://archives.postgresql.org/pgsql-bugs/2010-06/msg00211.php

PFA a patch to implement $SUBJECT. One interesting aspect of this
patch is that types like "integer" and "double precision" don't get
quoted in the output, whereas types like "text" do. But it turns out
that types like "integer" and "double precision" don't *work* if
they're quoted, so this is not a bad thing. It might possibly be
judged to require documentation somewhere, however.

Uh, I thought this was about quoting the identifiers. I am confused
about why "integer" is an issue in this case. Can you show an example?

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

+ None of us is going to be here forever. +

#3Robert Haas
robertmhaas@gmail.com
In reply to: Bruce Momjian (#2)
Re: pg_dump(all) --quote-all-identifiers

On Mon, Jun 14, 2010 at 6:57 AM, Bruce Momjian <bruce@momjian.us> wrote:

Robert Haas wrote:

In response to a complaint from Hartmut Goebel:

http://archives.postgresql.org/pgsql-bugs/2010-06/msg00018.php

And per a design proposed by Tom Lane:

http://archives.postgresql.org/pgsql-bugs/2010-06/msg00211.php

PFA a patch to implement $SUBJECT.  One interesting aspect of this
patch is that types like "integer" and "double precision" don't get
quoted in the output, whereas types like "text" do.  But it turns out
that types like "integer" and "double precision" don't *work* if
they're quoted, so this is not a bad thing.  It might possibly be
judged to require documentation somewhere, however.

Uh, I thought this was about quoting the identifiers.  I am confused
about why "integer" is an issue in this case.  Can you show an example?

Sure.

rhaas=# create table bruce (demonstration "integer");
ERROR: type "integer" does not exist
LINE 1: create table bruce (demonstration "integer");
^
rhaas=# create table bruce (demonstration integer);
CREATE TABLE

See gram.y, around line 8341.

Note that if you try the same example with "text" instead of
"integer", both variants work.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise Postgres Company

#4Tom Lane
tgl@sss.pgh.pa.us
In reply to: Robert Haas (#3)
Re: pg_dump(all) --quote-all-identifiers

Robert Haas <robertmhaas@gmail.com> writes:

On Mon, Jun 14, 2010 at 6:57 AM, Bruce Momjian <bruce@momjian.us> wrote:

Uh, I thought this was about quoting the identifiers. �I am confused
about why "integer" is an issue in this case. �Can you show an example?

Sure.

INTEGER is actually a keyword in this context, not an identifier.
(Remember the actual name of the type is int4, not integer.)

regards, tom lane

#5Alex Hunsaker
badalex@gmail.com
In reply to: Robert Haas (#1)
1 attachment(s)
Re: pg_dump(all) --quote-all-identifiers

On Sun, Jun 13, 2010 at 18:35, Robert Haas <robertmhaas@gmail.com> wrote:

*Waves* Hi!

Patch looks and tests good to me. Only thing that seemed to be
missing was documentation of the new pg_dump(all) and guc params.
Find attached a stab at this. Yeah the docs I added need work, but I
figure if you are anything like me its easier to work/tweak from a
(crappy) base... At least with docs :). I also bumped the version
check to 90100 from 90000.

For testing I dumped one of my production schemas that has over
something like 1000 relations and lots of indexs and junk. (~ 1.4MB
for a schema only dump). I also tried some custom types just because
I was curious.

Everything worked as expected.

PFA a patch to implement $SUBJECT.  One interesting aspect of this
patch is that types like "integer" and "double precision" don't get
quoted in the output, whereas types like "text" do.  But it turns out
that types like "integer" and "double precision" don't *work* if
they're quoted, so this is not a bad thing.  It might possibly be
judged to require documentation somewhere, however.

IMHO I don't think additional documentation for the above is needed.
*shrug* But it does make me wonder if there is some subtle way for it
to break if we somehow did call quote_ident with "integer". Not that
I saw anyway for this to happen...

Attachments:

quote_all_idents_v2.patch.gzapplication/x-gzip; name=quote_all_idents_v2.patch.gzDownload
#6Robert Haas
robertmhaas@gmail.com
In reply to: Alex Hunsaker (#5)
Re: pg_dump(all) --quote-all-identifiers

On Sat, Jul 17, 2010 at 3:59 AM, Alex Hunsaker <badalex@gmail.com> wrote:

Patch looks and tests good to me.  Only thing that seemed to be
missing was documentation of the new pg_dump(all) and guc params.
Find attached a stab at this.  Yeah the docs I added need work, but I
figure if you are anything like me its easier to work/tweak from a
(crappy) base...  At least with docs :).  I also bumped the version
check to 90100 from 90000.

Thanks for the review. Committed with some further cleanup.

Unfortunately, as I noticed just after committing, you only fixed ONE
of the two references to 90000, so I have corrected that with a second
commit.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise Postgres Company