Re: Storing pg_stat_statements query texts externally, pg_stat_statements in core

From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: Peter Geoghegan <pg(at)heroku(dot)com>
Cc: Fujii Masao <masao(dot)fujii(at)gmail(dot)com>, Peter Eisentraut <peter_e(at)gmx(dot)net>, Pg Hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: Storing pg_stat_statements query texts externally, pg_stat_statements in core
Date: 2014-01-25 19:04:29
Message-ID: 2080.1390676669@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Peter Geoghegan <pg(at)heroku(dot)com> writes:
> On Wed, Jan 22, 2014 at 1:22 PM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
>> BTW, have you got any sort of test scenario you could share for this
>> purpose? I'm sure I could build something, but if you've already
>> done it ...

> I simply ran the standard regression tests, and then straced a backend
> as it executed the pgss-calling query. I'm not sure that I've
> understood your question.

> As previously noted, my approach to testing this patch involved variations of:
> running "make installcheck-parallel"

Do the regression tests fail for you when doing this?

What I see is a duplicate occurrence of an escape_string_warning bleat:

*** /home/postgres/pgsql/src/test/regress/expected/plpgsql.out Fri Jan 3 17:07
:46 2014
--- /home/postgres/pgsql/src/test/regress/results/plpgsql.out Sat Jan 25 13:37
:20 2014
***************
*** 4568,4573 ****
--- 4568,4579 ----
HINT: Use the escape string syntax for backslashes, e.g., E'\\'.
QUERY: SELECT 'foo\\bar\041baz'
CONTEXT: PL/pgSQL function strtest() line 4 at RETURN
+ WARNING: nonstandard use of \\ in a string literal
+ LINE 1: SELECT 'foo\\bar\041baz'
+ ^
+ HINT: Use the escape string syntax for backslashes, e.g., E'\\'.
+ QUERY: SELECT 'foo\\bar\041baz'
+ CONTEXT: PL/pgSQL function strtest() line 4 at RETURN
strtest
-------------
foo\bar!baz

======================================================================

which seems to happen because generate_normalized_query() reruns the
core lexer on the given statement, and if you got a warning the first
time, you'll get another one.

It seems this only happens with rather small values of
pg_stat_statements.max, else there's already a "RETURN text-constant"
query in the hashtable so we don't need to do reparsing right here.
(I'm testing with max = 100 to exercise the garbage collection code.)

I'm not sure this is a big deal for real-world use, because surely by now
everyone has either fixed their escape-string issues or disabled the
warning. But it's annoying for testing.

The big picture of course is that having this warning issued this way
is broken anyhow, and has been since day one, so it's not entirely
pg_stat_statements' fault. I'm just wondering if it's worth taking
the trouble to turn off the warning when reparsing.

regards, tom lane

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Tomas Vondra 2014-01-25 19:44:30 Re: GIN improvements part2: fast scan
Previous Message Jeremy Harris 2014-01-25 18:25:19 Questionable coding in orderedsetaggs.c