Re: Re: pg_stat_statements normalisation without invasive changes to the parser (was: Next steps on pg_stat_statements normalisation)

On 27 March 2012 20:26, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> Have yet to look at the pg_stat_statements code itself.

I merged upstream changes with the intention of providing a new patch
for you to review. I found a problem that I'd guess was introduced by
commit 9dbf2b7d75de5af38d087cbe2b1147dd0fd10f0a, "Restructure SELECT
INTO's parsetree representation into CreateTableAsStmt". This has
nothing to do with my patch in particular.

I noticed that my tests broke, on queries like "select * into
orders_recent FROM orders WHERE orderdate >= '2002-01-01';". Since
commands like that are now utility statements, I thought it best to
just hash the query string directly, along with all other utility
statements - richer functionality would be unlikely to be missed all
that much, and that's a fairly clean demarcation that I don't want to
make blurry.

In the existing pg_stat_statements code in HEAD, there are 2
pgss_store call sites - one in pgss_ProcessUtility, and the other in
pgss_ExecutorFinish. There is an implicit assumption in the extant
code (and my patch too) that there will be exactly one pgss_store call
per query execution. However, that assumption appears to now fall
down, as illustrated by the GDB session below. What's more, our new
hook is called twice, which is arguably redundant.

(gdb) break pgss_parse_analyze
Breakpoint 1 at 0x7fbd17b96790: file pg_stat_statements.c, line 640.
(gdb) break pgss_ProcessUtility
Breakpoint 2 at 0x7fbd17b962b4: file pg_stat_statements.c, line 1710.
(gdb) break pgss_ExecutorEnd
Breakpoint 3 at 0x7fbd17b9618c: file pg_stat_statements.c, line 1674.
(gdb) c
Continuing.

< I execute the command "select * into orders_recent FROM orders WHERE
orderdate >= '2002-01-01';" >

Breakpoint 1, pgss_parse_analyze (pstate=0x2473bc0,
post_analysis_tree=0x2474010) at pg_stat_statements.c:640
640 if (post_analysis_tree->commandType != CMD_UTILITY)
(gdb) c
Continuing.

Breakpoint 1, pgss_parse_analyze (pstate=0x2473bc0,
post_analysis_tree=0x2474010) at pg_stat_statements.c:640
640 if (post_analysis_tree->commandType != CMD_UTILITY)
(gdb) c
Continuing.

Breakpoint 2, pgss_ProcessUtility (parsetree=0x2473cd8,
queryString=0x2472a88 "select * into orders_recent FROM orders WHERE
orderdate >= '2002-01-01';", params=0x0,
isTopLevel=1 '\001', dest=0x2474278, completionTag=0x7fff74e481e0
"") at pg_stat_statements.c:1710
1710 if (pgss_track_utility && pgss_enabled())
(gdb) c
Continuing.

Breakpoint 3, pgss_ExecutorEnd (queryDesc=0x24c9660) at
pg_stat_statements.c:1674
1674 if (queryDesc->totaltime && pgss_enabled())
(gdb) c
Continuing.

What do you think we should do about this?

--
Peter Geoghegan       http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training and Services