On 2019-Dec-10, Alvaro Herrera wrote: > On 2019-Dec-09, Tom Lane wrote: > > > Some quick review of v19: > > Here's v20 with all these comments hopefully addressed.
Grr, I had forgotten to git add the stringinfo.h -> pg_wchar.h changes, so the prototype isn't anywhere in v20. However, after looking at it again, I'm not very happy with how that turned out, because pg_wchar.h is a frontend-exposed header. So I instead propose to put it in a separate file src/include/mb/stringinfo_mb.h. -- Álvaro Herrera https://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
>From bd8df7f46556eb8a5de5e64f04b29f4d7a174d0e Mon Sep 17 00:00:00 2001 From: Alvaro Herrera <alvhe...@alvh.no-ip.org> Date: Tue, 10 Dec 2019 14:40:56 -0300 Subject: [PATCH v21] Add backend-only appendStringInfoStringQuoted MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This provides a mechanism to emit literal values in informative messages, such as query parameters. The new code is more complex than what it replaces, primarily because it wants to be more efficient. It also has the (currently unused) additional optional capability of specifying a maximum size to print. This lives separetely from common/stringinfo.c so that frontend users of that file need not pull in unnecessary multibyte-encoding support code. Author: Álvaro Herrera and Alexey Bashtanov, after a suggestion from Andres Freund Reviewed-by: Tom Lane Discussion: https://postgr.es/m/20190920203905.xkv5udsd5dxfs...@alap3.anarazel.de --- src/backend/tcop/postgres.c | 11 +--- src/backend/utils/mb/Makefile | 1 + src/backend/utils/mb/README | 1 + src/backend/utils/mb/stringinfo_mb.c | 92 +++++++++++++++++++++++++++ src/include/mb/stringinfo_mb.h | 24 +++++++ src/pl/plpgsql/src/pl_exec.c | 37 ++++------- src/test/regress/expected/plpgsql.out | 14 ++++ src/test/regress/sql/plpgsql.sql | 13 ++++ 8 files changed, 158 insertions(+), 35 deletions(-) create mode 100644 src/backend/utils/mb/stringinfo_mb.c create mode 100644 src/include/mb/stringinfo_mb.h diff --git a/src/backend/tcop/postgres.c b/src/backend/tcop/postgres.c index 3b85e48333..512209a38c 100644 --- a/src/backend/tcop/postgres.c +++ b/src/backend/tcop/postgres.c @@ -48,6 +48,7 @@ #include "libpq/pqformat.h" #include "libpq/pqsignal.h" #include "mb/pg_wchar.h" +#include "mb/stringinfo_mb.h" #include "miscadmin.h" #include "nodes/print.h" #include "optimizer/optimizer.h" @@ -2348,7 +2349,6 @@ errdetail_params(ParamListInfo params) Oid typoutput; bool typisvarlena; char *pstring; - char *p; appendStringInfo(¶m_str, "%s$%d = ", paramno > 0 ? ", " : "", @@ -2364,14 +2364,7 @@ errdetail_params(ParamListInfo params) pstring = OidOutputFunctionCall(typoutput, prm->value); - appendStringInfoCharMacro(¶m_str, '\''); - for (p = pstring; *p; p++) - { - if (*p == '\'') /* double single quotes */ - appendStringInfoCharMacro(¶m_str, *p); - appendStringInfoCharMacro(¶m_str, *p); - } - appendStringInfoCharMacro(¶m_str, '\''); + appendStringInfoStringQuoted(¶m_str, pstring, 0); pfree(pstring); } diff --git a/src/backend/utils/mb/Makefile b/src/backend/utils/mb/Makefile index 18dd758cfe..cd4a016449 100644 --- a/src/backend/utils/mb/Makefile +++ b/src/backend/utils/mb/Makefile @@ -16,6 +16,7 @@ OBJS = \ conv.o \ encnames.o \ mbutils.o \ + stringinfo_mb.o \ wchar.o \ wstrcmp.o \ wstrncmp.o diff --git a/src/backend/utils/mb/README b/src/backend/utils/mb/README index c9bc6e6f8d..7495ca5db2 100644 --- a/src/backend/utils/mb/README +++ b/src/backend/utils/mb/README @@ -9,6 +9,7 @@ wchar.c: mostly static functions and a public table for mb string and multibyte conversion mbutils.c: public functions for the backend only. requires conv.c and wchar.c +stringinfo_mb.c: public backend-only multibyte-aware stringinfo functions wstrcmp.c: strcmp for mb wstrncmp.c: strncmp for mb win866.c: a tool to generate KOI8 <--> CP866 conversion table diff --git a/src/backend/utils/mb/stringinfo_mb.c b/src/backend/utils/mb/stringinfo_mb.c new file mode 100644 index 0000000000..e84b9ad36f --- /dev/null +++ b/src/backend/utils/mb/stringinfo_mb.c @@ -0,0 +1,92 @@ +/*------------------------------------------------------------------------- + * + * stringinfo_mb.c + * Multibyte encoding-aware additional StringInfo facilites + * + * This is separate from common/stringinfo.c so that frontend users + * of that file need not pull in unnecessary multibyte-encoding support + * code. + * + * + * Portions Copyright (c) 1996-2019, PostgreSQL Global Development Group + * Portions Copyright (c) 1994, Regents of the University of California + * + * + * IDENTIFICATION + * src/backend/utils/mb/stringinfo_mb.c + * + *------------------------------------------------------------------------- + */ +#include "postgres.h" + +#include "mb/stringinfo_mb.h" +#include "mb/pg_wchar.h" + + +/* + * appendStringInfoStringQuoted + * + * Append up to maxlen characters from s to str, or the whole input string if + * maxlen <= 0, adding single quotes around it and doubling all single quotes. + * Add an ellipsis if the copy is incomplete. + */ +void +appendStringInfoStringQuoted(StringInfo str, const char *s, int maxlen) +{ + char *copy = NULL; + const char *chunk_search_start, + *chunk_copy_start, + *chunk_end; + bool ellipsis; + int slen; + + Assert(str != NULL); + + slen = strlen(s); + if (maxlen > 0 && maxlen < slen) + { + int finallen = pg_mbcliplen(s, slen, maxlen); + + copy = pnstrdup(s, finallen); + chunk_search_start = copy; + chunk_copy_start = copy; + + ellipsis = true; + } + else + { + chunk_search_start = s; + chunk_copy_start = s; + + ellipsis = false; + } + + appendStringInfoCharMacro(str, '\''); + + while ((chunk_end = strchr(chunk_search_start, '\'')) != NULL) + { + /* copy including the found delimiting ' */ + appendBinaryStringInfoNT(str, + chunk_copy_start, + chunk_end - chunk_copy_start + 1); + + /* in order to double it, include this ' into the next chunk as well */ + chunk_copy_start = chunk_end; + chunk_search_start = chunk_end + 1; + } + + /* copy the last chunk and terminate */ + if (ellipsis) + appendStringInfo(str, "%s...'", chunk_copy_start); + else + { + int len = strlen(chunk_copy_start); + + /* ensure sufficient space for terminators */ + appendBinaryStringInfoNT(str, chunk_copy_start, len); + appendStringInfoCharMacro(str, '\''); + } + + if (copy) + pfree(copy); +} diff --git a/src/include/mb/stringinfo_mb.h b/src/include/mb/stringinfo_mb.h new file mode 100644 index 0000000000..e90ba8bc51 --- /dev/null +++ b/src/include/mb/stringinfo_mb.h @@ -0,0 +1,24 @@ +/*------------------------------------------------------------------------- + * + * stringinfo_mb.h + * multibyte support for StringInfo + * + * Portions Copyright (c) 1996-2019, PostgreSQL Global Development Group + * Portions Copyright (c) 1994, Regents of the University of California + * + * src/include/mb/stringinfo_mb.h + *------------------------------------------------------------------------- + */ +#ifndef STRINGINFO_MB_H +#define STRINGINFO_MB_H + + +#include "lib/stringinfo.h" + +/* + * Multibyte-aware StringInfo support function. + */ +extern void appendStringInfoStringQuoted(StringInfo str, + const char *s, int maxlen); + +#endif /* STRINGINFO_MB_H */ diff --git a/src/pl/plpgsql/src/pl_exec.c b/src/pl/plpgsql/src/pl_exec.c index 4f0de7a811..2deb7c0b12 100644 --- a/src/pl/plpgsql/src/pl_exec.c +++ b/src/pl/plpgsql/src/pl_exec.c @@ -28,6 +28,7 @@ #include "executor/spi.h" #include "executor/spi_priv.h" #include "funcapi.h" +#include "mb/stringinfo_mb.h" #include "miscadmin.h" #include "nodes/nodeFuncs.h" #include "optimizer/optimizer.h" @@ -8611,19 +8612,11 @@ format_expr_params(PLpgSQL_execstate *estate, if (paramisnull) appendStringInfoString(¶mstr, "NULL"); else - { - char *value = convert_value_to_string(estate, paramdatum, paramtypeid); - char *p; - - appendStringInfoCharMacro(¶mstr, '\''); - for (p = value; *p; p++) - { - if (*p == '\'') /* double single quotes */ - appendStringInfoCharMacro(¶mstr, *p); - appendStringInfoCharMacro(¶mstr, *p); - } - appendStringInfoCharMacro(¶mstr, '\''); - } + appendStringInfoStringQuoted(¶mstr, + convert_value_to_string(estate, + paramdatum, + paramtypeid), + 0); paramno++; } @@ -8661,19 +8654,11 @@ format_preparedparamsdata(PLpgSQL_execstate *estate, if (ppd->nulls[paramno] == 'n') appendStringInfoString(¶mstr, "NULL"); else - { - char *value = convert_value_to_string(estate, ppd->values[paramno], ppd->types[paramno]); - char *p; - - appendStringInfoCharMacro(¶mstr, '\''); - for (p = value; *p; p++) - { - if (*p == '\'') /* double single quotes */ - appendStringInfoCharMacro(¶mstr, *p); - appendStringInfoCharMacro(¶mstr, *p); - } - appendStringInfoCharMacro(¶mstr, '\''); - } + appendStringInfoStringQuoted(¶mstr, + convert_value_to_string(estate, + ppd->values[paramno], + ppd->types[paramno]), + 0); } MemoryContextSwitchTo(oldcontext); diff --git a/src/test/regress/expected/plpgsql.out b/src/test/regress/expected/plpgsql.out index e85b29455e..cd2c79f4d5 100644 --- a/src/test/regress/expected/plpgsql.out +++ b/src/test/regress/expected/plpgsql.out @@ -2656,6 +2656,20 @@ create or replace function stricttest() returns void as $$ declare x record; p1 int := 2; +p3 text := $a$'Valame Dios!' dijo Sancho; 'no le dije yo a vuestra merced que mirase bien lo que hacia?'$a$; +begin + -- no rows + select * from foo where f1 = p1 and f1::text = p3 into strict x; + raise notice 'x.f1 = %, x.f2 = %', x.f1, x.f2; +end$$ language plpgsql; +select stricttest(); +ERROR: query returned no rows +DETAIL: parameters: p1 = '2', p3 = '''Valame Dios!'' dijo Sancho; ''no le dije yo a vuestra merced que mirase bien lo que hacia?''' +CONTEXT: PL/pgSQL function stricttest() line 8 at SQL statement +create or replace function stricttest() returns void as $$ +declare +x record; +p1 int := 2; p3 text := 'foo'; begin -- too many rows diff --git a/src/test/regress/sql/plpgsql.sql b/src/test/regress/sql/plpgsql.sql index 70deadfbea..d841d8c0f9 100644 --- a/src/test/regress/sql/plpgsql.sql +++ b/src/test/regress/sql/plpgsql.sql @@ -2280,6 +2280,19 @@ end$$ language plpgsql; select stricttest(); +create or replace function stricttest() returns void as $$ +declare +x record; +p1 int := 2; +p3 text := $a$'Valame Dios!' dijo Sancho; 'no le dije yo a vuestra merced que mirase bien lo que hacia?'$a$; +begin + -- no rows + select * from foo where f1 = p1 and f1::text = p3 into strict x; + raise notice 'x.f1 = %, x.f2 = %', x.f1, x.f2; +end$$ language plpgsql; + +select stricttest(); + create or replace function stricttest() returns void as $$ declare x record; -- 2.20.1
