Skip site navigation (1) Skip section navigation (2)

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

From: Peter Geoghegan <peter(at)2ndquadrant(dot)com>
To: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Cc: Daniel Farina <daniel(at)heroku(dot)com>, Robert Haas <robertmhaas(at)gmail(dot)com>, PG Hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: Re: pg_stat_statements normalisation without invasive changes to the parser (was: Next steps on pg_stat_statements normalisation)
Date: 2012-03-28 14:12:31
Message-ID: CAEYLb_X=1=Ov8ZUjxrG-PM=RfP1xwjyDqmvao_zc4NVb9kUr+g@mail.gmail.com (view raw or flat)
Thread:
Lists: pgsql-hackers
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

In response to

Responses

pgsql-hackers by date

Next:From: Tom LaneDate: 2012-03-28 14:15:58
Subject: Re: [PATCH] Lazy hashaggregate when no aggregation is needed
Previous:From: Tom LaneDate: 2012-03-28 14:02:21
Subject: Re: Cross-backend signals and administration (Was: Re: pg_terminate_backend for same-role)

Privacy Policy | About PostgreSQL
Copyright © 1996-2014 The PostgreSQL Global Development Group