On Sat, Nov 8, 2014 at 5:55 AM, Alvaro Herrera <[email protected]> wrote:
> Michael Paquier wrote:
>> On Fri, Nov 7, 2014 at 2:31 AM, Alvaro Herrera <[email protected]>
>> wrote:
>
>> > I thought the consensus was that the SQL-callable function declarations
>> > should remain in builtins.h -- mainly so that quote.h does not need to
>> > include fmgr.h.
>> Moving everything to quote.h is done in-line with what Tom and Robert
>> suggested, builtins.h being a refuge for function declarations that
>> have nowhere else to go. Suggestion from here:
>> http://www.postgresql.org/message-id/ca+tgmozf3dkptua6ex6gxlnnd-nms-fbjcxroitwffh-+6y...@mail.gmail.com
>
> Did you miss this one?
> http://www.postgresql.org/message-id/[email protected]
Well, yes :) I missed that. Note that I am leaning to Robert's
direction as well to do a clear separation... Now if the final
consensus is different, then let's use the patch attached that puts
the SQL functions to builtins.h, and the rest in quote.h.
Regards,
--
Michael
From f69abec98e96dcbaff3476a07a9d6536d9d5b5e5 Mon Sep 17 00:00:00 2001
From: Michael Paquier <[email protected]>
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/tablefunc/tablefunc.c | 1 +
contrib/test_decoding/test_decoding.c | 1 +
contrib/worker_spi/worker_spi.c | 2 +-
src/backend/catalog/namespace.c | 1 +
src/backend/catalog/objectaddress.c | 1 +
src/backend/commands/explain.c | 1 +
src/backend/commands/matview.c | 2 +-
src/backend/commands/trigger.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 | 27 +++++++++
src/pl/plpgsql/src/pl_gram.y | 1 +
src/pl/plpython/plpy_plpymodule.c | 1 +
24 files changed, 158 insertions(+), 116 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/tablefunc/tablefunc.c b/contrib/tablefunc/tablefunc.c
index 10ee8c7..66e5ccf 100644
--- a/contrib/tablefunc/tablefunc.c
+++ b/contrib/tablefunc/tablefunc.c
@@ -41,6 +41,7 @@
#include "lib/stringinfo.h"
#include "miscadmin.h"
#include "utils/builtins.h"
+#include "utils/quote.h"
#include "tablefunc.h"
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..ec6c9fa 100644
--- a/contrib/worker_spi/worker_spi.c
+++ b/contrib/worker_spi/worker_spi.c
@@ -37,7 +37,7 @@
#include "fmgr.h"
#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 911f015..76a50ac 100644
--- a/src/backend/catalog/namespace.c
+++ b/src/backend/catalog/namespace.c
@@ -54,6 +54,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 99aa0f0..2ca305c 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/matview.c b/src/backend/commands/matview.c
index 523ba35..83900b4 100644
--- a/src/backend/commands/matview.c
+++ b/src/backend/commands/matview.c
@@ -34,8 +34,8 @@
#include "storage/lmgr.h"
#include "storage/smgr.h"
#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 31a5411..093c32c 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/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..82574ca 100644
--- a/src/backend/utils/adt/format_type.c
+++ b/src/backend/utils/adt/format_type.c
@@ -23,6 +23,7 @@
#include "utils/builtins.h"
#include "utils/lsyscache.h"
#include "utils/numeric.h"
+#include "utils/quote.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 bf4e81f..87305da 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
@@ -8957,111 +8955,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..67c0e3b 100644
--- a/src/backend/utils/adt/varlena.c
+++ b/src/backend/utils/adt/varlena.c
@@ -30,6 +30,7 @@
#include "utils/lsyscache.h"
#include "utils/memutils.h"
#include "utils/pg_locale.h"
+#include "utils/quote.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 aca4243..6ae0a37 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 4e74d85..fa59d04 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..f141fc7
--- /dev/null
+++ b/src/include/utils/quote.h
@@ -0,0 +1,27 @@
+/*-------------------------------------------------------------------------
+ *
+ * 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;
+
+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.3
--
Sent via pgsql-hackers mailing list ([email protected])
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
