On 2019-Dec-09, Tom Lane wrote: > Some quick review of v19:
Here's v20 with all these comments hopefully addressed. -- Álvaro Herrera https://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
>From 62277fcd3f63ae68495300c98f77c7e4b4713614 Mon Sep 17 00:00:00 2001 From: Alvaro Herrera <alvhe...@alvh.no-ip.org> Date: Tue, 3 Dec 2019 10:08:35 -0300 Subject: [PATCH v20] Add backend-only appendStringInfoStringQuoted MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Simplifies some coding that prints parameters, as well as optimize to do it per non-quote chunks instead of per byte. 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 | 10 +-- src/backend/utils/mb/Makefile | 1 + src/backend/utils/mb/README | 1 + src/backend/utils/mb/stringinfo_mb.c | 92 +++++++++++++++++++++++++++ src/pl/plpgsql/src/pl_exec.c | 36 +++-------- src/test/regress/expected/plpgsql.out | 14 ++++ src/test/regress/sql/plpgsql.sql | 13 ++++ 7 files changed, 132 insertions(+), 35 deletions(-) create mode 100644 src/backend/utils/mb/stringinfo_mb.c diff --git a/src/backend/tcop/postgres.c b/src/backend/tcop/postgres.c index 3b85e48333..3d3172e83d 100644 --- a/src/backend/tcop/postgres.c +++ b/src/backend/tcop/postgres.c @@ -2348,7 +2348,6 @@ errdetail_params(ParamListInfo params) Oid typoutput; bool typisvarlena; char *pstring; - char *p; appendStringInfo(¶m_str, "%s$%d = ", paramno > 0 ? ", " : "", @@ -2364,14 +2363,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..baef1aa39a --- /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 "lib/stringinfo.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/pl/plpgsql/src/pl_exec.c b/src/pl/plpgsql/src/pl_exec.c index 4f0de7a811..c3ae409f39 100644 --- a/src/pl/plpgsql/src/pl_exec.c +++ b/src/pl/plpgsql/src/pl_exec.c @@ -8611,19 +8611,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 +8653,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