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

From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: Peter Geoghegan <peter(at)2ndquadrant(dot)com>
Cc: 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-29 02:39:33
Message-ID: 9914.1332988773@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

BTW, I forgot to mention that I did experiment with your python-based
test script for pg_stat_statements, but decided not to commit it.
There are just too many external dependencies for my taste:

1. python
2. psycopg2
3. dellstore2 test database

That coupled with the apparent impossibility of running the script
without manual preconfiguration makes it look not terribly useful.

Also, as of the committed patch there are several individual tests that
fail or need adjustment:

The SELECT INTO tests all fail, but we know the reason why (the testbed
isn't expecting them to result in creating separate entries for the
utility statement and the underlying plannable SELECT).

verify_statement_equivalency("select a.orderid from orders a join orders b on a.orderid = b.orderid",
"select b.orderid from orders a join orders b on a.orderid = b.orderid", conn)

These are not equivalent statements, or at least would not be if the
join condition were anything else than what it is, so the fact that the
original coding failed to distinguish the targetlist entries is a bug.

The test
# temporary column name within recursive CTEs doesn't differentiate
fails, not because of the change of column name, but because of the
change of CTE name. This is a consequence of my having used the CTE
name here:

case RTE_CTE:

/*
* Depending on the CTE name here isn't ideal, but it's the
* only info we have to identify the referenced WITH item.
*/
APP_JUMB_STRING(rte->ctename);
APP_JUMB(rte->ctelevelsup);
break;

We could avoid the name dependency by omitting ctename from the jumble
but I think that cure is worse than the disease.

Anyway, not too important, but I just thought I'd document this in case
you were wondering about the discrepancies.

regards, tom lane

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Tom Lane 2012-03-29 02:45:00 Re: Re: [COMMITTERS] pgsql: pg_test_timing utility, to measure clock monotonicity and timing
Previous Message Fujii Masao 2012-03-29 02:33:04 Re: Re: [COMMITTERS] pgsql: pg_test_timing utility, to measure clock monotonicity and timing