split builtins.h to quote.h - Mailing list pgsql-hackers
From | Alvaro Herrera |
---|---|
Subject | split builtins.h to quote.h |
Date | |
Msg-id | 20141011134439.GP7043@eldon.alvh.no-ip.org Whole thread Raw |
Responses |
Re: split builtins.h to quote.h
|
List | pgsql-hackers |
Started a new thread to raise awareness. Michael Paquier wrote: > On Thu, Oct 9, 2014 at 8:29 AM, Michael Paquier > <michael.paquier@gmail.com> wrote: > > On Thu, Oct 9, 2014 at 6:26 AM, Alvaro Herrera <alvherre@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. If someone wants to object to this, please speak quickly. Ref: this comes from http://www.postgresql.org/message-id/CAB7nPqR1iVd5r_QN_ngmkBOLQmAGBOsJ4WNPo8eybNn6WE_Kdw@mail.gmail.com > From b14000662a52c3f5b40cf43a1f6699e0d024d1fd 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/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 > -- Álvaro Herrera http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training & Services
pgsql-hackers by date: