Re: Storing pg_stat_statements query texts externally, pg_stat_statements in core - Mailing list pgsql-hackers

From Tom Lane
Subject Re: Storing pg_stat_statements query texts externally, pg_stat_statements in core
Date
Msg-id 2080.1390676669@sss.pgh.pa.us
Whole thread Raw
In response to Re: Storing pg_stat_statements query texts externally, pg_stat_statements in core  (Peter Geoghegan <pg@heroku.com>)
Responses Re: Storing pg_stat_statements query texts externally, pg_stat_statements in core  (Peter Geoghegan <pg@heroku.com>)
List pgsql-hackers
Peter Geoghegan <pg@heroku.com> writes:
> On Wed, Jan 22, 2014 at 1:22 PM, Tom Lane <tgl@sss.pgh.pa.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



pgsql-hackers by date:

Previous
From: Jeremy Harris
Date:
Subject: Questionable coding in orderedsetaggs.c
Next
From: Tomas Vondra
Date:
Subject: Re: GIN improvements part2: fast scan