On Thu, Oct 9, 2014 at 8:29 AM, Michael Paquier
<michael.paqu...@gmail.com> wrote:
> On Thu, Oct 9, 2014 at 6:26 AM, Alvaro Herrera <alvhe...@2ndquadrant.com>
> wrote:
>> However, if I were to do it, I would instead create a quote.h file and
>> would also add the quote_literal_cstr() prototype to it, perhaps even
>> move the implementations from their current ruleutils.c location to
>> quote.c. (Why is half the stuff in ruleutils.c rather than quote.c is
>> beyond me.)
>
> Yes, an extra header file would be cleaner. Now if we are worried about
> creating much incompatibilities with existing extension (IMO that's not
> something core should worry much about), then it is better not to do it.
> Now, if I can vote on it, I think it is worth doing in order to have
> builtins.h only contain only Datum foo(PG_FUNCTION_ARGS), and move all the
> quote-related things in quote.c.
The attached patch implements this idea, creating a new header quote.h
in include/utils that groups all the quote-related functions. This
makes the code more organized.
Regards,
--
Michael
From b14000662a52c3f5b40cf43a1f6699e0d024d1fd Mon Sep 17 00:00:00 2001
From: Michael Paquier <mich...@otacoo.com>
Date: Sat, 11 Oct 2014 22:28:05 +0900
Subject: [PATCH] Move all quote-related functions into a single header quote.h
Note that this impacts as well contrib modules, and mainly external
modules particularly for quote_identifier.
---
contrib/dblink/dblink.c | 1 +
contrib/postgres_fdw/deparse.c | 1 +
contrib/postgres_fdw/postgres_fdw.c | 1 +
contrib/test_decoding/test_decoding.c | 1 +
contrib/worker_spi/worker_spi.c | 1 +
src/backend/catalog/namespace.c | 1 +
src/backend/catalog/objectaddress.c | 1 +
src/backend/commands/explain.c | 1 +
src/backend/commands/extension.c | 1 +
src/backend/commands/matview.c | 1 +
src/backend/commands/trigger.c | 1 +
src/backend/commands/tsearchcmds.c | 1 +
src/backend/parser/parse_utilcmd.c | 1 +
src/backend/utils/adt/format_type.c | 1 +
src/backend/utils/adt/quote.c | 110 ++++++++++++++++++++++++++++++++++
src/backend/utils/adt/regproc.c | 1 +
src/backend/utils/adt/ri_triggers.c | 1 +
src/backend/utils/adt/ruleutils.c | 109 +--------------------------------
src/backend/utils/adt/varlena.c | 1 +
src/backend/utils/cache/ts_cache.c | 1 +
src/backend/utils/misc/guc.c | 1 +
src/include/utils/builtins.h | 6 --
src/include/utils/quote.h | 28 +++++++++
src/pl/plpgsql/src/pl_gram.y | 1 +
src/pl/plpython/plpy_plpymodule.c | 1 +
25 files changed, 160 insertions(+), 114 deletions(-)
create mode 100644 src/include/utils/quote.h
diff --git a/contrib/dblink/dblink.c b/contrib/dblink/dblink.c
index d77b3ee..cbbc513 100644
--- a/contrib/dblink/dblink.c
+++ b/contrib/dblink/dblink.c
@@ -56,6 +56,7 @@
#include "utils/guc.h"
#include "utils/lsyscache.h"
#include "utils/memutils.h"
+#include "utils/quote.h"
#include "utils/rel.h"
#include "utils/tqual.h"
diff --git a/contrib/postgres_fdw/deparse.c b/contrib/postgres_fdw/deparse.c
index 2045774..0009cd3 100644
--- a/contrib/postgres_fdw/deparse.c
+++ b/contrib/postgres_fdw/deparse.c
@@ -50,6 +50,7 @@
#include "parser/parsetree.h"
#include "utils/builtins.h"
#include "utils/lsyscache.h"
+#include "utils/quote.h"
#include "utils/syscache.h"
diff --git a/contrib/postgres_fdw/postgres_fdw.c b/contrib/postgres_fdw/postgres_fdw.c
index 4c49776..feb41f5 100644
--- a/contrib/postgres_fdw/postgres_fdw.c
+++ b/contrib/postgres_fdw/postgres_fdw.c
@@ -36,6 +36,7 @@
#include "utils/guc.h"
#include "utils/lsyscache.h"
#include "utils/memutils.h"
+#include "utils/quote.h"
PG_MODULE_MAGIC;
diff --git a/contrib/test_decoding/test_decoding.c b/contrib/test_decoding/test_decoding.c
index fdbd313..b168fc3 100644
--- a/contrib/test_decoding/test_decoding.c
+++ b/contrib/test_decoding/test_decoding.c
@@ -25,6 +25,7 @@
#include "utils/builtins.h"
#include "utils/lsyscache.h"
#include "utils/memutils.h"
+#include "utils/quote.h"
#include "utils/rel.h"
#include "utils/relcache.h"
#include "utils/syscache.h"
diff --git a/contrib/worker_spi/worker_spi.c b/contrib/worker_spi/worker_spi.c
index 328c722..7ada98d 100644
--- a/contrib/worker_spi/worker_spi.c
+++ b/contrib/worker_spi/worker_spi.c
@@ -38,6 +38,7 @@
#include "lib/stringinfo.h"
#include "pgstat.h"
#include "utils/builtins.h"
+#include "utils/quote.h"
#include "utils/snapmgr.h"
#include "tcop/utility.h"
diff --git a/src/backend/catalog/namespace.c b/src/backend/catalog/namespace.c
index 5eb8fd4..7f8f54b 100644
--- a/src/backend/catalog/namespace.c
+++ b/src/backend/catalog/namespace.c
@@ -53,6 +53,7 @@
#include "utils/inval.h"
#include "utils/lsyscache.h"
#include "utils/memutils.h"
+#include "utils/quote.h"
#include "utils/syscache.h"
diff --git a/src/backend/catalog/objectaddress.c b/src/backend/catalog/objectaddress.c
index b69b75b..d7a1ec7 100644
--- a/src/backend/catalog/objectaddress.c
+++ b/src/backend/catalog/objectaddress.c
@@ -74,6 +74,7 @@
#include "utils/builtins.h"
#include "utils/fmgroids.h"
#include "utils/lsyscache.h"
+#include "utils/quote.h"
#include "utils/syscache.h"
#include "utils/tqual.h"
diff --git a/src/backend/commands/explain.c b/src/backend/commands/explain.c
index 49963ff..f27ad15 100644
--- a/src/backend/commands/explain.c
+++ b/src/backend/commands/explain.c
@@ -27,6 +27,7 @@
#include "utils/builtins.h"
#include "utils/json.h"
#include "utils/lsyscache.h"
+#include "utils/quote.h"
#include "utils/rel.h"
#include "utils/ruleutils.h"
#include "utils/snapmgr.h"
diff --git a/src/backend/commands/extension.c b/src/backend/commands/extension.c
index 9a0afa4..0793509 100644
--- a/src/backend/commands/extension.c
+++ b/src/backend/commands/extension.c
@@ -51,6 +51,7 @@
#include "utils/builtins.h"
#include "utils/fmgroids.h"
#include "utils/lsyscache.h"
+#include "utils/quote.h"
#include "utils/rel.h"
#include "utils/snapmgr.h"
#include "utils/tqual.h"
diff --git a/src/backend/commands/matview.c b/src/backend/commands/matview.c
index d1c8bb0..f6f3a04 100644
--- a/src/backend/commands/matview.c
+++ b/src/backend/commands/matview.c
@@ -35,6 +35,7 @@
#include "tcop/tcopprot.h"
#include "utils/builtins.h"
#include "utils/lsyscache.h"
+#include "utils/quote.h"
#include "utils/rel.h"
#include "utils/snapmgr.h"
#include "utils/syscache.h"
diff --git a/src/backend/commands/trigger.c b/src/backend/commands/trigger.c
index f4c0ffa..df62484 100644
--- a/src/backend/commands/trigger.c
+++ b/src/backend/commands/trigger.c
@@ -52,6 +52,7 @@
#include "utils/inval.h"
#include "utils/lsyscache.h"
#include "utils/memutils.h"
+#include "utils/quote.h"
#include "utils/rel.h"
#include "utils/snapmgr.h"
#include "utils/syscache.h"
diff --git a/src/backend/commands/tsearchcmds.c b/src/backend/commands/tsearchcmds.c
index 5d8a001..40758cc 100644
--- a/src/backend/commands/tsearchcmds.c
+++ b/src/backend/commands/tsearchcmds.c
@@ -42,6 +42,7 @@
#include "utils/builtins.h"
#include "utils/fmgroids.h"
#include "utils/lsyscache.h"
+#include "utils/quote.h"
#include "utils/rel.h"
#include "utils/syscache.h"
#include "utils/tqual.h"
diff --git a/src/backend/parser/parse_utilcmd.c b/src/backend/parser/parse_utilcmd.c
index 7c1939f..a991872 100644
--- a/src/backend/parser/parse_utilcmd.c
+++ b/src/backend/parser/parse_utilcmd.c
@@ -57,6 +57,7 @@
#include "utils/acl.h"
#include "utils/builtins.h"
#include "utils/lsyscache.h"
+#include "utils/quote.h"
#include "utils/rel.h"
#include "utils/syscache.h"
#include "utils/typcache.h"
diff --git a/src/backend/utils/adt/format_type.c b/src/backend/utils/adt/format_type.c
index e1763a3..02733c0 100644
--- a/src/backend/utils/adt/format_type.c
+++ b/src/backend/utils/adt/format_type.c
@@ -22,6 +22,7 @@
#include "catalog/pg_type.h"
#include "utils/builtins.h"
#include "utils/lsyscache.h"
+#include "utils/quote.h"
#include "utils/numeric.h"
#include "utils/syscache.h"
#include "mb/pg_wchar.h"
diff --git a/src/backend/utils/adt/quote.c b/src/backend/utils/adt/quote.c
index d1336b4..6fd983e 100644
--- a/src/backend/utils/adt/quote.c
+++ b/src/backend/utils/adt/quote.c
@@ -13,8 +13,13 @@
*/
#include "postgres.h"
+#include "lib/stringinfo.h"
+#include "parser/keywords.h"
#include "utils/builtins.h"
+#include "utils/quote.h"
+/* GUC parameters */
+bool quote_all_identifiers = false;
/*
* quote_ident -
@@ -95,6 +100,111 @@ quote_literal(PG_FUNCTION_ARGS)
}
/*
+ * quote_identifier - Quote an identifier only if needed
+ *
+ * When quotes are needed, we palloc the required space; slightly
+ * space-wasteful but well worth it for notational simplicity.
+ */
+const char *
+quote_identifier(const char *ident)
+{
+ /*
+ * Can avoid quoting if ident starts with a lowercase letter or underscore
+ * and contains only lowercase letters, digits, and underscores, *and* is
+ * not any SQL keyword. Otherwise, supply quotes.
+ */
+ int nquotes = 0;
+ bool safe;
+ const char *ptr;
+ char *result;
+ char *optr;
+
+ /*
+ * would like to use <ctype.h> macros here, but they might yield unwanted
+ * locale-specific results...
+ */
+ safe = ((ident[0] >= 'a' && ident[0] <= 'z') || ident[0] == '_');
+
+ for (ptr = ident; *ptr; ptr++)
+ {
+ char ch = *ptr;
+
+ if ((ch >= 'a' && ch <= 'z') ||
+ (ch >= '0' && ch <= '9') ||
+ (ch == '_'))
+ {
+ /* okay */
+ }
+ else
+ {
+ safe = false;
+ if (ch == '"')
+ nquotes++;
+ }
+ }
+
+ if (quote_all_identifiers)
+ safe = false;
+
+ if (safe)
+ {
+ /*
+ * Check for keyword. We quote keywords except for unreserved ones.
+ * (In some cases we could avoid quoting a col_name or type_func_name
+ * keyword, but it seems much harder than it's worth to tell that.)
+ *
+ * Note: ScanKeywordLookup() does case-insensitive comparison, but
+ * that's fine, since we already know we have all-lower-case.
+ */
+ const ScanKeyword *keyword = ScanKeywordLookup(ident,
+ ScanKeywords,
+ NumScanKeywords);
+
+ if (keyword != NULL && keyword->category != UNRESERVED_KEYWORD)
+ safe = false;
+ }
+
+ if (safe)
+ return ident; /* no change needed */
+
+ result = (char *) palloc(strlen(ident) + nquotes + 2 + 1);
+
+ optr = result;
+ *optr++ = '"';
+ for (ptr = ident; *ptr; ptr++)
+ {
+ char ch = *ptr;
+
+ if (ch == '"')
+ *optr++ = '"';
+ *optr++ = ch;
+ }
+ *optr++ = '"';
+ *optr = '\0';
+
+ return result;
+}
+
+/*
+ * quote_qualified_identifier - Quote a possibly-qualified identifier
+ *
+ * Return a name of the form qualifier.ident, or just ident if qualifier
+ * is NULL, quoting each component if necessary. The result is palloc'd.
+ */
+char *
+quote_qualified_identifier(const char *qualifier,
+ const char *ident)
+{
+ StringInfoData buf;
+
+ initStringInfo(&buf);
+ if (qualifier)
+ appendStringInfo(&buf, "%s.", quote_identifier(qualifier));
+ appendStringInfoString(&buf, quote_identifier(ident));
+ return buf.data;
+}
+
+/*
* quote_literal_cstr -
* returns a properly quoted literal
*/
diff --git a/src/backend/utils/adt/regproc.c b/src/backend/utils/adt/regproc.c
index c0314ee..3760bbc 100644
--- a/src/backend/utils/adt/regproc.c
+++ b/src/backend/utils/adt/regproc.c
@@ -38,6 +38,7 @@
#include "utils/builtins.h"
#include "utils/fmgroids.h"
#include "utils/lsyscache.h"
+#include "utils/quote.h"
#include "utils/syscache.h"
#include "utils/tqual.h"
diff --git a/src/backend/utils/adt/ri_triggers.c b/src/backend/utils/adt/ri_triggers.c
index c0156fa..c954b60 100644
--- a/src/backend/utils/adt/ri_triggers.c
+++ b/src/backend/utils/adt/ri_triggers.c
@@ -49,6 +49,7 @@
#include "utils/inval.h"
#include "utils/lsyscache.h"
#include "utils/memutils.h"
+#include "utils/quote.h"
#include "utils/rel.h"
#include "utils/snapmgr.h"
#include "utils/syscache.h"
diff --git a/src/backend/utils/adt/ruleutils.c b/src/backend/utils/adt/ruleutils.c
index 24ade6c..6521db9 100644
--- a/src/backend/utils/adt/ruleutils.c
+++ b/src/backend/utils/adt/ruleutils.c
@@ -54,6 +54,7 @@
#include "utils/builtins.h"
#include "utils/fmgroids.h"
#include "utils/lsyscache.h"
+#include "utils/quote.h"
#include "utils/rel.h"
#include "utils/ruleutils.h"
#include "utils/snapmgr.h"
@@ -273,9 +274,6 @@ static const char *query_getrulebyoid = "SELECT * FROM pg_catalog.pg_rewrite WHE
static SPIPlanPtr plan_getviewrule = NULL;
static const char *query_getviewrule = "SELECT * FROM pg_catalog.pg_rewrite WHERE ev_class = $1 AND rulename = $2";
-/* GUC parameters */
-bool quote_all_identifiers = false;
-
/* ----------
* Local functions
@@ -8887,111 +8885,6 @@ printSubscripts(ArrayRef *aref, deparse_context *context)
}
/*
- * quote_identifier - Quote an identifier only if needed
- *
- * When quotes are needed, we palloc the required space; slightly
- * space-wasteful but well worth it for notational simplicity.
- */
-const char *
-quote_identifier(const char *ident)
-{
- /*
- * Can avoid quoting if ident starts with a lowercase letter or underscore
- * and contains only lowercase letters, digits, and underscores, *and* is
- * not any SQL keyword. Otherwise, supply quotes.
- */
- int nquotes = 0;
- bool safe;
- const char *ptr;
- char *result;
- char *optr;
-
- /*
- * would like to use <ctype.h> macros here, but they might yield unwanted
- * locale-specific results...
- */
- safe = ((ident[0] >= 'a' && ident[0] <= 'z') || ident[0] == '_');
-
- for (ptr = ident; *ptr; ptr++)
- {
- char ch = *ptr;
-
- if ((ch >= 'a' && ch <= 'z') ||
- (ch >= '0' && ch <= '9') ||
- (ch == '_'))
- {
- /* okay */
- }
- else
- {
- safe = false;
- if (ch == '"')
- nquotes++;
- }
- }
-
- if (quote_all_identifiers)
- safe = false;
-
- if (safe)
- {
- /*
- * Check for keyword. We quote keywords except for unreserved ones.
- * (In some cases we could avoid quoting a col_name or type_func_name
- * keyword, but it seems much harder than it's worth to tell that.)
- *
- * Note: ScanKeywordLookup() does case-insensitive comparison, but
- * that's fine, since we already know we have all-lower-case.
- */
- const ScanKeyword *keyword = ScanKeywordLookup(ident,
- ScanKeywords,
- NumScanKeywords);
-
- if (keyword != NULL && keyword->category != UNRESERVED_KEYWORD)
- safe = false;
- }
-
- if (safe)
- return ident; /* no change needed */
-
- result = (char *) palloc(strlen(ident) + nquotes + 2 + 1);
-
- optr = result;
- *optr++ = '"';
- for (ptr = ident; *ptr; ptr++)
- {
- char ch = *ptr;
-
- if (ch == '"')
- *optr++ = '"';
- *optr++ = ch;
- }
- *optr++ = '"';
- *optr = '\0';
-
- return result;
-}
-
-/*
- * quote_qualified_identifier - Quote a possibly-qualified identifier
- *
- * Return a name of the form qualifier.ident, or just ident if qualifier
- * is NULL, quoting each component if necessary. The result is palloc'd.
- */
-char *
-quote_qualified_identifier(const char *qualifier,
- const char *ident)
-{
- StringInfoData buf;
-
- initStringInfo(&buf);
- if (qualifier)
- appendStringInfo(&buf, "%s.", quote_identifier(qualifier));
- appendStringInfoString(&buf, quote_identifier(ident));
- return buf.data;
-}
-
-/*
* get_relation_name
* Get the unqualified name of a relation specified by OID
*
diff --git a/src/backend/utils/adt/varlena.c b/src/backend/utils/adt/varlena.c
index c3171b5..eb4c507 100644
--- a/src/backend/utils/adt/varlena.c
+++ b/src/backend/utils/adt/varlena.c
@@ -28,6 +28,7 @@
#include "utils/builtins.h"
#include "utils/bytea.h"
#include "utils/lsyscache.h"
+#include "utils/quote.h"
#include "utils/memutils.h"
#include "utils/pg_locale.h"
#include "utils/sortsupport.h"
diff --git a/src/backend/utils/cache/ts_cache.c b/src/backend/utils/cache/ts_cache.c
index 5ff1461..c6d539f 100644
--- a/src/backend/utils/cache/ts_cache.c
+++ b/src/backend/utils/cache/ts_cache.c
@@ -45,6 +45,7 @@
#include "utils/inval.h"
#include "utils/lsyscache.h"
#include "utils/memutils.h"
+#include "utils/quote.h"
#include "utils/syscache.h"
#include "utils/tqual.h"
diff --git a/src/backend/utils/misc/guc.c b/src/backend/utils/misc/guc.c
index 8111b93..3253235 100644
--- a/src/backend/utils/misc/guc.c
+++ b/src/backend/utils/misc/guc.c
@@ -79,6 +79,7 @@
#include "utils/plancache.h"
#include "utils/portal.h"
#include "utils/ps_status.h"
+#include "utils/quote.h"
#include "utils/snapmgr.h"
#include "utils/tzparser.h"
#include "utils/xml.h"
diff --git a/src/include/utils/builtins.h b/src/include/utils/builtins.h
index fb1b4a4..c2df489 100644
--- a/src/include/utils/builtins.h
+++ b/src/include/utils/builtins.h
@@ -666,7 +666,6 @@ extern Datum record_image_ge(PG_FUNCTION_ARGS);
extern Datum btrecordimagecmp(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);
@@ -689,10 +688,6 @@ extern Datum pg_get_function_arguments(PG_FUNCTION_ARGS);
extern Datum pg_get_function_identity_arguments(PG_FUNCTION_ARGS);
extern Datum pg_get_function_result(PG_FUNCTION_ARGS);
extern Datum pg_get_function_arg_default(PG_FUNCTION_ARGS);
-extern const char *quote_identifier(const char *ident);
-extern char *quote_qualified_identifier(const char *qualifier,
- const char *ident);
-
/* tid.c */
extern Datum tidin(PG_FUNCTION_ARGS);
@@ -1072,7 +1067,6 @@ extern int32 type_maximum_size(Oid type_oid, int32 typemod);
/* quote.c */
extern Datum quote_ident(PG_FUNCTION_ARGS);
extern Datum quote_literal(PG_FUNCTION_ARGS);
-extern char *quote_literal_cstr(const char *rawstr);
extern Datum quote_nullable(PG_FUNCTION_ARGS);
/* guc.c */
diff --git a/src/include/utils/quote.h b/src/include/utils/quote.h
new file mode 100644
index 0000000..ccf0d83
--- /dev/null
+++ b/src/include/utils/quote.h
@@ -0,0 +1,28 @@
+/*-------------------------------------------------------------------------
+ *
+ * quote.h
+ * Declarations for operations related to string quoting.
+ *
+ *
+ * Portions Copyright (c) 1996-2014, PostgreSQL Global Development Group
+ * Portions Copyright (c) 1994, Regents of the University of California
+ *
+ * src/include/utils/quote.h
+ *
+ *-------------------------------------------------------------------------
+ */
+#ifndef QUOTE_H
+#define QUOTE_H
+
+#include "fmgr.h"
+
+/* GUC parameter */
+extern bool quote_all_identifiers;
+
+/* Functions used for quoting */
+extern const char *quote_identifier(const char *ident);
+extern char *quote_qualified_identifier(const char *qualifier,
+ const char *ident);
+extern char *quote_literal_cstr(const char *rawstr);
+
+#endif /* QUOTE_H */
diff --git a/src/pl/plpgsql/src/pl_gram.y b/src/pl/plpgsql/src/pl_gram.y
index 893f3a4..8e9d0a0 100644
--- a/src/pl/plpgsql/src/pl_gram.y
+++ b/src/pl/plpgsql/src/pl_gram.y
@@ -22,6 +22,7 @@
#include "parser/scanner.h"
#include "parser/scansup.h"
#include "utils/builtins.h"
+#include "utils/quote.h"
/* Location tracking support --- simpler than bison's default */
diff --git a/src/pl/plpython/plpy_plpymodule.c b/src/pl/plpython/plpy_plpymodule.c
index 37ea2a4..39e70f8 100644
--- a/src/pl/plpython/plpy_plpymodule.c
+++ b/src/pl/plpython/plpy_plpymodule.c
@@ -8,6 +8,7 @@
#include "mb/pg_wchar.h"
#include "utils/builtins.h"
+#include "utils/quote.h"
#include "plpython.h"
--
2.1.2
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
