On Sat, Nov 8, 2014 at 5:55 AM, Alvaro Herrera <alvhe...@2ndquadrant.com> wrote: > Michael Paquier wrote: >> On Fri, Nov 7, 2014 at 2:31 AM, Alvaro Herrera <alvhe...@2ndquadrant.com> >> 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/31728.1413209...@sss.pgh.pa.us 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 <michael@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/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 (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers