Re: [HACKERS] pg_stat_statements query normalization, and the 'in' operator

unixway.dr...@gmail.com writes: > Given the following list of queries: >create table foo (id serial, bar integer); >select * from foo where id in (1); >select * from foo where id in (2,3); >select * from foo where id in (1,3,5); >select * from foo where id in (select id from

[HACKERS] pg_stat_statements query normalization, and the 'in' operator

Hello there, Given the following list of queries: create table foo (id serial, bar integer); select * from foo where id in (1); select * from foo where id in (2,3); select * from foo where id in (1,3,5); select * from foo where id in (select id from foo); would it be possible to have

Re: [HACKERS] pg_stat_statements and non default search_path

On 16/10/2016 11:21, Craig Ringer wrote: > On 16 Oct. 2016 14:31, Julien Rouhaud wrote: >> >> On 16/10/2016 02:38, Jim Nasby wrote: >> > On 10/10/16 12:58 AM, Julien Rouhaud wrote: >> >> Unless you mean deparsing the Query instead of using raw source > text? I >> >> think that would solve this

Re: [HACKERS] pg_stat_statements and non default search_path

On 16 Oct. 2016 14:31, "Julien Rouhaud" wrote: > > On 16/10/2016 02:38, Jim Nasby wrote: > > On 10/10/16 12:58 AM, Julien Rouhaud wrote: > >> Unless you mean deparsing the Query instead of using raw source text? I > >> think that would solve this issue (and also the

Re: [HACKERS] pg_stat_statements and non default search_path

On 16/10/2016 10:47, Lukas Fittl wrote: > Can somebody chime in if it would be feasible to store this in the > out-of-band query text file, and whether a patch for this would be > considered acceptable? FWIW I already have a quick and dirty patch for this, I don't think there's any major

Re: [HACKERS] pg_stat_statements and non default search_path

On Sat, Oct 15, 2016 at 11:29 PM, Julien Rouhaud wrote: > > > BTW, after thinking about it some more, I don't see how storing the > > search_path would help at all... it's not like you can do anything with > > it unless you have a huge chunk of the parser available. > >

Re: [HACKERS] pg_stat_statements and non default search_path

On 16/10/2016 02:38, Jim Nasby wrote: > On 10/10/16 12:58 AM, Julien Rouhaud wrote: >> Unless you mean deparsing the Query instead of using raw source text? I >> think that would solve this issue (and also the other issue when >> multiple queries are submitted at once, you get the normalized

Re: [HACKERS] pg_stat_statements and non default search_path

On 10/10/16 12:58 AM, Julien Rouhaud wrote: > Would another option be to temporarily set search_path to '' when > getting the query text? ISTM that would be the best option. I don't think that possible. The query text used is raw query text provided by the post_parse_analyse hook

Re: [HACKERS] pg_stat_statements and non default search_path

On 10/10/2016 05:00, Jim Nasby wrote: > On 10/7/16 4:39 AM, Julien Rouhaud wrote: >> I see two ways of fixing this. First one would be to store normalized >> queries while fully qualifiying the relation's names. After a quick look >> this can't be done without storing at least a token location in

Re: [HACKERS] pg_stat_statements and non default search_path

On 10/7/16 4:39 AM, Julien Rouhaud wrote: I see two ways of fixing this. First one would be to store normalized queries while fully qualifiying the relation's names. After a quick look this can't be done without storing at least a token location in RangeTblEntry nodes, which sounds like a bad

[HACKERS] pg_stat_statements and non default search_path

Hello, I've faced multiple times a painful limitation with pg_stat_statements. Queries are normalized based on the textual SQL statements, and the queryid is computed using objects' oids. So for instance with different search_path settings, we can end up with the same normalized query but

Re: [HACKERS] pg_stat_statements query jumbling question

On Sun, Nov 15, 2015 at 1:34 AM, Tom Lane wrote: > Michael Paquier writes: >> On Sun, Nov 1, 2015 at 2:03 AM, Julien Rouhaud >> wrote: >>> I'm also rather sceptical about this change. > >> Hm. Thinking a bit about this

Re: [HACKERS] pg_stat_statements query jumbling question

Michael Paquier writes: > On Sun, Nov 1, 2015 at 2:03 AM, Julien Rouhaud > wrote: >> I'm also rather sceptical about this change. > Hm. Thinking a bit about this patch, it presents the advantage to be > able to track the same queries easily

Re: [HACKERS] pg_stat_statements query jumbling question

On Sun, Nov 1, 2015 at 2:03 AM, Julien Rouhaud wrote: > -BEGIN PGP SIGNED MESSAGE- > Hash: SHA1 > > Hello, > > On 10/10/2015 08:46, Satoshi Nagayasu wrote: >> On 2015/10/03 6:18, Peter Geoghegan wrote: >>> On Wed, Sep 2, 2015 at 7:41 PM, Satoshi Nagayasu >>>

Re: [HACKERS] pg_stat_statements query jumbling question

On Sat, Oct 31, 2015 at 10:03 AM, Julien Rouhaud wrote: >> At least, I would like to give some options to be chosen by the >> user. Is it possible and/or reasonable? >> > > I'm also rather sceptical about this change. Is anyone willing to argue for it, apart from

Re: [HACKERS] pg_stat_statements query jumbling question

-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 Hello, On 10/10/2015 08:46, Satoshi Nagayasu wrote: > On 2015/10/03 6:18, Peter Geoghegan wrote: >> On Wed, Sep 2, 2015 at 7:41 PM, Satoshi Nagayasu >> wrote: >>> I know this still needs to be discussed, but I would like to >>>

Re: [HACKERS] pg_stat_statements query jumbling question

On 2015/10/03 6:18, Peter Geoghegan wrote: On Wed, Sep 2, 2015 at 7:41 PM, Satoshi Nagayasu wrote: I know this still needs to be discussed, but I would like to submit a patch for further discussion at the next CF, 2015-11. I think I already expressed this in my explanation

Re: [HACKERS] pg_stat_statements query jumbling question

On Wed, Sep 2, 2015 at 7:41 PM, Satoshi Nagayasu wrote: > I know this still needs to be discussed, but I would like to submit > a patch for further discussion at the next CF, 2015-11. I think I already expressed this in my explanation of the current behavior, but to be clear: -1

Re: [HACKERS] pg_stat_statements query jumbling question

On 2015/09/01 14:39, Satoshi Nagayasu wrote: > On 2015/09/01 14:01, Tom Lane wrote: >> Satoshi Nagayasu writes: >>> On 2015/09/01 13:41, Peter Geoghegan wrote: If you want to use the queryId field directly, which I recall you mentioning before, then that's harder. There

[HACKERS] pg_stat_statements query jumbling question

Hi, I have a question on jumbling queries in pg_stat_statements. I found that JumbleRangeTable() uses relation oid in RangeTblEntry. Obviously, this would result different queryid when the table gets re-created (dropped and created). Why don't we use relation name (with looking up the catalog)

Re: [HACKERS] pg_stat_statements query jumbling question

On Mon, Aug 31, 2015 at 8:32 PM, Satoshi Nagayasu wrote: > Why don't we use relation name (with looking up the catalog) > on query jumbling? For performance reason? I think that there is a good case for preferring this behavior. While it is a little confusing that

Re: [HACKERS] pg_stat_statements query jumbling question

On Mon, Aug 31, 2015 at 9:29 PM, Satoshi Nagayasu wrote: > BTW, I'm interested in improving the queryid portability now because > I'd like to use it in other extensions. :) > That's the reason why I'm looking at query jumbling here. Are you interested in having the query

Re: [HACKERS] pg_stat_statements query jumbling question

On 2015/09/01 13:41, Peter Geoghegan wrote: On Mon, Aug 31, 2015 at 9:29 PM, Satoshi Nagayasu wrote: BTW, I'm interested in improving the queryid portability now because I'd like to use it in other extensions. :) That's the reason why I'm looking at query jumbling here. Are

Re: [HACKERS] pg_stat_statements query jumbling question

On 2015/09/01 14:01, Tom Lane wrote: > Satoshi Nagayasu writes: >> On 2015/09/01 13:41, Peter Geoghegan wrote: >>> If you want to use the queryId field directly, which I recall you >>> mentioning before, then that's harder. There is simply no contract >>> among extensions for

Re: [HACKERS] pg_stat_statements query jumbling question

On 2015/09/01 12:36, Peter Geoghegan wrote: On Mon, Aug 31, 2015 at 8:32 PM, Satoshi Nagayasu wrote: Why don't we use relation name (with looking up the catalog) on query jumbling? For performance reason? I think that there is a good case for preferring this behavior.

Re: [HACKERS] pg_stat_statements query jumbling question

Satoshi Nagayasu writes: > On 2015/09/01 13:41, Peter Geoghegan wrote: >> If you want to use the queryId field directly, which I recall you >> mentioning before, then that's harder. There is simply no contract >> among extensions for "owning" a queryId. But when the

[HACKERS] pg_stat_statements vs escape_string_warning

I happened to notice that if you run the regression tests with pg_stat_statements installed, you will often (not always) get a failure that looks like this: *** src/test/regress/expected/plpgsql.out Tue Jan 20 12:01:52 2015 --- src/test/regress/results/plpgsql.out Wed Jan 21 12:43:19 2015

Re: [HACKERS] pg_stat_statements vs escape_string_warning

Peter Geoghegan p...@heroku.com writes: On Wed, Jan 21, 2015 at 10:14 AM, Tom Lane t...@sss.pgh.pa.us wrote: This isn't a back-patchable bug fix, but given the lack of prior complaints, maybe it doesn't matter. Alternatively, we could back-patch only the addition of escape_string_warning to

Re: [HACKERS] pg_stat_statements vs escape_string_warning

On Wed, Jan 21, 2015 at 10:14 AM, Tom Lane t...@sss.pgh.pa.us wrote: What I'm inclined to do about this is add an escape_string_warning bool field to struct core_yy_extra_type, which would be copied from the GUC variable by scanner_init(); then check_string_escape_warning() would consult that

Re: [HACKERS] pg_stat_statements cluttered with DEALLOCATE dbdpg_p*

On 07/20/2014 11:51 PM, Peter Geoghegan wrote: On Sun, Jul 20, 2014 at 10:56 AM, Tom Lane t...@sss.pgh.pa.us wrote: However, this is certainly a behavioral change. Perhaps squeeze it into 9.4, but not the back braches? +1 Ok, done. (We're a month closer to releasing 9.4 than we were when

Re: [HACKERS] pg_stat_statements cluttered with DEALLOCATE dbdpg_p*

Hello devs, I noticed that my pg_stat_statements is cluttered with hundreds of entries like DEALLOCATE dbdpg_p123456_7, occuring each only once. Here is a patch and sql test file to: * normalize DEALLOCATE utility statements in pg_stat_statements Some drivers such as DBD:Pg generate

Re: [HACKERS] pg_stat_statements cluttered with DEALLOCATE dbdpg_p*

Hi, On 2014-04-01 16:45:29 +0200, Fabien COELHO wrote: I noticed that my pg_stat_statements is cluttered with hundreds of entries like DEALLOCATE dbdpg_p123456_7, occuring each only once. Why isn't the driver using the extended query protocol? Sending PREPARE/EXECUTE/DEALLOCATE wastes

Re: [HACKERS] pg_stat_statements cluttered with DEALLOCATE dbdpg_p*

On 2014-07-20 13:54:01 +0200, Andres Freund wrote: Hi, On 2014-04-01 16:45:29 +0200, Fabien COELHO wrote: I noticed that my pg_stat_statements is cluttered with hundreds of entries like DEALLOCATE dbdpg_p123456_7, occuring each only once. Why isn't the driver using the extended query

Re: [HACKERS] pg_stat_statements cluttered with DEALLOCATE dbdpg_p*

On 2014-07-20 14:06, Andres Freund wrote: On 2014-07-20 13:54:01 +0200, Andres Freund wrote: On 2014-04-01 16:45:29 +0200, Fabien COELHO wrote: I noticed that my pg_stat_statements is cluttered with hundreds of entries like DEALLOCATE dbdpg_p123456_7, occuring each only once. Why isn't the

Re: [HACKERS] pg_stat_statements cluttered with DEALLOCATE dbdpg_p*

Hello Andres, Why isn't the driver using the extended query protocol? Sending PREPARE/EXECUTE/DEALLOCATE wastes roundtrips... It seems to me that it would be more helful if these similar entries where aggregated together, that is if the query normalization could ignore the name of the

Re: [HACKERS] pg_stat_statements cluttered with DEALLOCATE dbdpg_p*

On 2014-07-20 14:43:27 +0200, Fabien COELHO wrote: a) Consider using the extended query protocol. b) consider using unnamed prepared statements to reduce the number of roundtrips c) wonder why PREPARE/DEALLOCATE are so much more frequent than the actualy query execution. (1) I'm not

Re: [HACKERS] pg_stat_statements cluttered with DEALLOCATE dbdpg_p*

That's because PREPARE isn't executed as it's own statement, but done on the protocol level (which will need noticeably fewer messages). There's no builtin logic to ignore actual PREPARE statements. ISTM that there is indeed a special handling in function pgss_ProcessUtility for PREPARE and

Re: [HACKERS] pg_stat_statements cluttered with DEALLOCATE dbdpg_p*

That's because PREPARE isn't executed as it's own statement, but done on the protocol level (which will need noticeably fewer messages). There's no builtin logic to ignore actual PREPARE statements. ISTM that there is indeed a special handling in function pgss_ProcessUtility for PREPARE and

Re: [HACKERS] pg_stat_statements cluttered with DEALLOCATE dbdpg_p*

On 2014-07-20 17:01:50 +0200, Fabien COELHO wrote: That's because PREPARE isn't executed as it's own statement, but done on the protocol level (which will need noticeably fewer messages). There's no builtin logic to ignore actual PREPARE statements. ISTM that there is indeed a special

Re: [HACKERS] pg_stat_statements cluttered with DEALLOCATE dbdpg_p*

[...]. If we do something we should go for the !IsA(parsetree, DeallocateStmt), not the normalization. Ok. The latter is pretty darn bogus. Yep:-) I'm fine with ignoring DEALLOCATE altogether. -- Fabien. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make

Re: [HACKERS] pg_stat_statements cluttered with DEALLOCATE dbdpg_p*

Andres Freund and...@2ndquadrant.com writes: On 2014-07-20 17:01:50 +0200, Fabien COELHO wrote: If you do not like my normalization hack (I do not like it much either:-), I have suggested to add !IsA(parsetree, DeallocateStmt) to the condition above, which would ignore DEALLOCATE as PREPARE

Re: [HACKERS] pg_stat_statements cluttered with DEALLOCATE dbdpg_p*

If you do not like my normalization hack (I do not like it much either:-), I have suggested to add !IsA(parsetree, DeallocateStmt) to the condition above, which would ignore DEALLOCATE as PREPARE and EXECUTE are currently and rightfully ignored. Well, EXECUTE isn't actually ignored, but

Re: [HACKERS] pg_stat_statements cluttered with DEALLOCATE dbdpg_p*

On Sun, Jul 20, 2014 at 10:56 AM, Tom Lane t...@sss.pgh.pa.us wrote: However, this is certainly a behavioral change. Perhaps squeeze it into 9.4, but not the back braches? +1 -- Peter Geoghegan -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your

[HACKERS] pg_stat_statements: Query normalisation may fail during stats reset

Hi, when regularly collecting resetting query information from pg_stat_statements it’s possible to trigger a situation where unnormalised queries are stored. I think what happens is the following: pgss_post_parse_analyse calls pgss_store with a non-null jstate which will cause the query

Re: [HACKERS] pg_stat_statements: Query normalisation may fail during stats reset

On Tue, May 6, 2014 at 12:26 PM, Michael Renner michael.ren...@amd.co.at wrote: when regularly collecting resetting query information from pg_stat_statements it’s possible to trigger a situation where unnormalised queries are stored. I think what happens is the following:

Re: [HACKERS] pg_stat_statements: Query normalisation may fail during stats reset

Robert Haas robertmh...@gmail.com writes: On Tue, May 6, 2014 at 12:26 PM, Michael Renner when regularly collecting resetting query information from pg_stat_statements it’s possible to trigger a situation where unnormalised queries are stored. Is this something that should be fixed or is

Re: [HACKERS] pg_stat_statements: Query normalisation may fail during stats reset

On Tue, May 6, 2014 at 11:31 AM, Tom Lane t...@sss.pgh.pa.us wrote: The source code says that query strings are normalized on a best effort basis, so perhaps we ought to say the same in the documentation. Perhaps. It would be rather expensive to provide a guarantee of normalization:

[HACKERS] pg_stat_statements cluttered with DEALLOCATE dbdpg_p*

Hello pgdevs, I noticed that my pg_stat_statements is cluttered with hundreds of entries like DEALLOCATE dbdpg_p123456_7, occuring each only once. It seems to me that it would be more helful if these similar entries where aggregated together, that is if the query normalization could ignore

Re: [HACKERS] pg_stat_statements cluttered with DEALLOCATE dbdpg_p*

On 04/01/2014 10:45 AM, Fabien COELHO wrote: Hello pgdevs, I noticed that my pg_stat_statements is cluttered with hundreds of entries like DEALLOCATE dbdpg_p123456_7, occuring each only once. It seems to me that it would be more helful if these similar entries where aggregated together,

Re: [HACKERS] pg_stat_statements cluttered with DEALLOCATE dbdpg_p*

I noticed that my pg_stat_statements is cluttered with hundreds of entries like DEALLOCATE dbdpg_p123456_7, occuring each only once. It seems to me that it would be more helful if these similar entries where aggregated together, that is if the query normalization could ignore the name of

[HACKERS] pg_stat_statements shows too short COMMIT time

Hello, I've found a problem with pg_stat_statements while doing some performance test. If the fix is desired and not difficult, I'm willing to address it. Could you give me any information and/or your opinions? [Problem] The time of COMMIT statements is unreasonably short.

[HACKERS] pg_stat_statements fingerprinting logic and ArrayExpr

pg_stat_statements' fingerprinting logic considers the following two statements as distinct: select 1 in (1, 2, 3); select 1 in (1, 2, 3, 4); This is because the ArrayExpr jumble case jumbles any ArrayExpr's list of elements recursively. In this case it's a list of Const nodes, and the

Re: [HACKERS] pg_stat_statements fingerprinting logic and ArrayExpr

On Tue, Dec 10, 2013 at 4:30 AM, Peter Geoghegan p...@heroku.com wrote: pg_stat_statements' fingerprinting logic considers the following two statements as distinct: select 1 in (1, 2, 3); select 1 in (1, 2, 3, 4); This is because the ArrayExpr jumble case jumbles any ArrayExpr's list of

Re: [HACKERS] pg_stat_statements fingerprinting logic and ArrayExpr

On Tue, Dec 10, 2013 at 1:09 PM, Robert Haas robertmh...@gmail.com wrote: I am very wary of implementing special-case logic here even though I know it could be useful to some people, simply because I fear that there could be a near-infinite variety of situations where, in a particular

Re: [HACKERS] pg_stat_statements fingerprinting logic and ArrayExpr

On 2013-12-10 16:09:02 -0500, Robert Haas wrote: On Tue, Dec 10, 2013 at 4:30 AM, Peter Geoghegan p...@heroku.com wrote: I'm not sure that I agree, but there is anecdata that suggests that it isn't uncommon for these sorts of queries to be broken out when they're all traceable back to a

Re: [HACKERS] pg_stat_statements fingerprinting logic and ArrayExpr

Robert Haas robertmh...@gmail.com writes: On Tue, Dec 10, 2013 at 4:30 AM, Peter Geoghegan p...@heroku.com wrote: pg_stat_statements' fingerprinting logic considers the following two statements as distinct: select 1 in (1, 2, 3); select 1 in (1, 2, 3, 4); [ and some people think it

Re: [HACKERS] pg_stat_statements fingerprinting logic and ArrayExpr

On Tue, Dec 10, 2013 at 1:41 PM, Andres Freund and...@2ndquadrant.com wrote: FWIW, I hit exactly this issue every single time I have looked at pg_stat_statements in some client's database making it nearly useless for analysis. So improving it might be worthwile. I think the thing that makes me

Re: [HACKERS] pg_stat_statements fingerprinting logic and ArrayExpr

On 2013-12-10 14:30:36 -0800, Peter Geoghegan wrote: Did you really find pg_stat_statements to be almost useless in such situations? That seems worse than I thought. It's very hard to see where you should spend efforts when every logical query is split into hundreds of pg_stat_statement

Re: [HACKERS] pg_stat_statements fingerprinting logic and ArrayExpr

On Tue, Dec 10, 2013 at 2:38 PM, Andres Freund and...@2ndquadrant.com wrote: It's very hard to see where you should spend efforts when every logical query is split into hundreds of pg_stat_statement entries. Suddenly it's important whether a certain counts of parameters are more frequent than

Re: [HACKERS] pg_stat_statements fingerprinting logic and ArrayExpr

On Tue, Dec 10, 2013 at 5:38 PM, Andres Freund and...@2ndquadrant.com wrote: On 2013-12-10 14:30:36 -0800, Peter Geoghegan wrote: Did you really find pg_stat_statements to be almost useless in such situations? That seems worse than I thought. It's very hard to see where you should spend

Re: [HACKERS] pg_stat_statements fingerprinting logic and ArrayExpr

On Tue, Dec 10, 2013 at 2:46 PM, Robert Haas robertmh...@gmail.com wrote: Right, but the flip side is that you could collapse things that people don't want collapsed. If you've got lots of query that differ only in that some of them say user_id IN (const1, const2) and others say user_id IN

Re: [HACKERS] pg_stat_statements fingerprinting logic and ArrayExpr

On 2013-12-10 17:46:56 -0500, Robert Haas wrote: On Tue, Dec 10, 2013 at 5:38 PM, Andres Freund and...@2ndquadrant.com wrote: On 2013-12-10 14:30:36 -0800, Peter Geoghegan wrote: Did you really find pg_stat_statements to be almost useless in such situations? That seems worse than I thought.

Re: [HACKERS] pg_stat_statements fingerprinting logic and ArrayExpr

On Tue, Dec 10, 2013 at 2:55 PM, Peter Geoghegan p...@heroku.com wrote: You might get lucky and have this exact case, and be able to leverage the knowledge that the 2 constants in the ArrayExpr must be the latter and 1 must be the former, but experience suggests very probably not. When things

Re: [HACKERS] pg_stat_statements fingerprinting logic and ArrayExpr

Robert Haas robertmh...@gmail.com writes: Right, but the flip side is that you could collapse things that people don't want collapsed. If you've got lots of query that differ only in that some of them say user_id IN (const1, const2) and others say user_id IN (const1, const2, const3) and the

Re: [HACKERS] pg_stat_statements fingerprinting logic and ArrayExpr

On Tue, Dec 10, 2013 at 3:08 PM, Tom Lane t...@sss.pgh.pa.us wrote: So my objection to what Peter is suggesting is not that it's a bad idea in isolation, but that I don't see where he's going to stop, short of reinventing every query-normalization behavior that exists in the planner. If this

Re: [HACKERS] pg_stat_statements fingerprinting logic and ArrayExpr

On Tue, Dec 10, 2013 at 3:08 PM, Tom Lane t...@sss.pgh.pa.us wrote: A different point of view is that it's more or less an implementation artifact that pg_stat_statements doesn't already see the cases as equivalent; that happens only because it looks at the querytree before the planner gets

Re: [HACKERS] pg_stat_statements fingerprinting logic and ArrayExpr

Peter Geoghegan p...@heroku.com writes: On Tue, Dec 10, 2013 at 3:08 PM, Tom Lane t...@sss.pgh.pa.us wrote: I'm wondering whether this doesn't indicate that we need to rethink where the fingerprinter has been plugged in. I'm not sure that somewhere in the planner, post-constant-folding, would

Re: [HACKERS] pg_stat_statements: calls under-estimation propagation

On Sun, Dec 8, 2013 at 1:00 AM, Peter Geoghegan p...@heroku.com wrote: On Sat, Dec 7, 2013 at 3:50 PM, Peter Eisentraut pete...@gmx.net wrote: 32-bit buildfarm members are having problems with this patch. This should fix that problem. Thanks. Applied. I also noted on

Re: [HACKERS] pg_stat_statements: calls under-estimation propagation

On Sat, 2013-12-07 at 16:00 -0800, Peter Geoghegan wrote: On Sat, Dec 7, 2013 at 3:50 PM, Peter Eisentraut pete...@gmx.net wrote: 32-bit buildfarm members are having problems with this patch. This should fix that problem. Thanks. This is incidentally the same problem that was reported here

Re: [HACKERS] pg_stat_statements: calls under-estimation propagation

On Sat, Dec 7, 2013 at 6:31 AM, Peter Geoghegan p...@heroku.com wrote: On Fri, Dec 6, 2013 at 12:24 PM, Tom Lane t...@sss.pgh.pa.us wrote: There seems to be no problem even if we use bigint as the type of unsigned 32-bit integer like queryid. For example, txid_current() returns the transaction

Re: [HACKERS] pg_stat_statements: calls under-estimation propagation

On Sun, 2013-12-08 at 02:08 +0900, Fujii Masao wrote: Attached revision displays signed 64-bit integers instead. Thanks! Looks good to me. Committed! 32-bit buildfarm members are having problems with this patch. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To

Re: [HACKERS] pg_stat_statements: calls under-estimation propagation

On Sat, Dec 7, 2013 at 3:50 PM, Peter Eisentraut pete...@gmx.net wrote: 32-bit buildfarm members are having problems with this patch. This should fix that problem. Thanks. -- Peter Geoghegan diff --git a/contrib/pg_stat_statements/pg_stat_statements.c

Re: [HACKERS] pg_stat_statements: calls under-estimation propagation

On Sun, Nov 24, 2013 at 10:58 AM, Peter Geoghegan p...@heroku.com wrote: On Mon, Nov 18, 2013 at 1:54 AM, Sameer Thakur samthaku...@gmail.com wrote: Please find v10 of patch attached. This patch addresses following review comments I've cleaned this up - revision attached - and marked it ready

Re: [HACKERS] pg_stat_statements: calls under-estimation propagation

Fujii Masao masao.fu...@gmail.com writes: On Sun, Nov 24, 2013 at 10:58 AM, Peter Geoghegan p...@heroku.com wrote: I decided that queryid should be of type oid, not bigint. This is arguably a slight abuse of notation, but since ultimately Oids are just abstract object identifiers (so say the

Re: [HACKERS] pg_stat_statements: calls under-estimation propagation

On Fri, Dec 6, 2013 at 12:24 PM, Tom Lane t...@sss.pgh.pa.us wrote: There seems to be no problem even if we use bigint as the type of unsigned 32-bit integer like queryid. For example, txid_current() returns the transaction ID, i.e., unsigned 32-bit integer, as bigint. Could you tell me what

Re: [HACKERS] pg_stat_statements: calls under-estimation propagation

I've cleaned this up - revision attached - and marked it ready for committer. Thank you for this. I did the basic hygiene test. The patch applies correctly and compiles with no warnings. Did not find anything broken in basic functionality. In the documentation i have a minor suggestion of

Re: [HACKERS] pg_stat_statements: calls under-estimation propagation

On Mon, Nov 18, 2013 at 1:54 AM, Sameer Thakur samthaku...@gmail.com wrote: Please find v10 of patch attached. This patch addresses following review comments I've cleaned this up - revision attached - and marked it ready for committer. I decided that queryid should be of type oid, not bigint.

Re: [HACKERS] pg_stat_statements: calls under-estimation propagation

Hello, Please find v10 of patch attached. This patch addresses following review comments 1. Removed errcode and used elogs for error pg_stat_statements schema is not supported by its binary 2. Removed comments and other code formatting not directly relevant to patch functionality 3. changed

Re: [HACKERS] pg_stat_statements: calls under-estimation propagation

On Tue, Nov 5, 2013 at 5:30 AM, Sameer Thakur samthaku...@gmail.com wrote: Hello, Please find attached pg_stat_statements-identification-v9.patch. I took a quick look. Observations: + /* Making query ID dependent on PG version */ + query-queryId |= PG_VERSION_NUM 16; If you want

Re: [HACKERS] pg_stat_statements: calls under-estimation propagation

On Thu, Nov 14, 2013 at 7:25 PM, Peter Geoghegan p...@heroku.com wrote: On Tue, Nov 5, 2013 at 5:30 AM, Sameer Thakur samthaku...@gmail.com wrote: Hello, Please find attached pg_stat_statements-identification-v9.patch. I took a quick look. Observations: + /* Making query ID dependent

Re: [HACKERS] pg_stat_statements: calls under-estimation propagation

I took a quick look. Observations: + /* Making query ID dependent on PG version */ + query-queryId |= PG_VERSION_NUM 16; If you want to do something like this, make the value of PGSS_FILE_HEADER incorporate (PG_VERSION_NUM / 100) or something. Why are you doing this? The thought was

Re: [HACKERS] pg_stat_statements: calls under-estimation propagation

Hello, Please find attached pg_stat_statements-identification-v9.patch. I have tried to address the following review comments 1. Use version PGSS_TUP_V1_2 2.Fixed total time being zero 3. Remove 'session_start' from the view and use point release number to generate queryid 4. Hide only queryid and

Re: [HACKERS] pg_stat_statements: calls under-estimation propagation

This paragraph reads a bit strange to me: + A statistics session is the time period when statistics are gathered by statistics collector + without being reset. So a statistics session continues across normal shutdowns, + but whenever statistics are reset, like during a crash or upgrade,

Re: [HACKERS] pg_stat_statements: calls under-estimation propagation

On 10/10/13 6:20 AM, Sameer Thakur wrote: Please find patch attached which adds documentation for session_start and introduced fields and corrects documentation for queryid to be query_id. session_start remains in the view as agreed. Please fix the tabs in the SGML files. -- Sent via

Re: [HACKERS] pg_stat_statements: calls under-estimation propagation

The V7-Patch applied cleanly and I got no issues in my first tests. The change from column session_start to a function seems very reasonable for me. Concernig the usability, I would like to suggest a minor change, that massively increases the usefulness of the patch for beginners, who often

Re: [HACKERS] pg_stat_statements: calls under-estimation propagation

Please find patch attached which adds documentation for session_start and introduced fields and corrects documentation for queryid to be query_id. session_start remains in the view as agreed. regards Sameer pg_stat_statements-identification-v8.patch.gz Description: GNU Zip compressed data --

Re: [HACKERS] pg_stat_statements: calls under-estimation propagation

On Thu, Oct 10, 2013 at 3:11 AM, pilum...@uni-muenster.de wrote: But the drawback of this approach is impossibility to use explain analyze without further substitutions. You can fairly easily disable the swapping of constants with '?' symbols, so that the query text stored would match the full

Re: [HACKERS] pg_stat_statements: calls under-estimation propagation

Thx for your reply. On Thu, 10 Oct 2013, Peter Geoghegan wrote: On Thu, Oct 10, 2013 at 3:11 AM, pilum...@uni-muenster.de wrote: But the drawback of this approach is impossibility to use explain analyze without further substitutions. You can fairly easily disable the swapping of constants

Re: [HACKERS] pg_stat_statements: calls under-estimation propagation

On Thu, Oct 10, 2013 at 7:20 PM, Sameer Thakur samthaku...@gmail.com wrote: Please find patch attached which adds documentation for session_start and introduced fields and corrects documentation for queryid to be query_id. session_start remains in the view as agreed. Thanks for updating the

Re: [HACKERS] pg_stat_statements: calls under-estimation propagation

On Thu, Oct 10, 2013 at 7:40 AM, Fujii Masao masao.fu...@gmail.com wrote: On Thu, Oct 10, 2013 at 7:20 PM, Sameer Thakur samthaku...@gmail.com wrote: Please find patch attached which adds documentation for session_start and introduced fields and corrects documentation for queryid to be

Re: [HACKERS] pg_stat_statements: calls under-estimation propagation

On Fri, Oct 11, 2013 at 12:19 AM, Daniel Farina dan...@heroku.com wrote: Probably. The idea is that without those fields it's, to wit, impossible to explain non-monotonic movement in metrics of those queries for precise use in tools that insist on monotonicity of the fields, which are all

Re: [HACKERS] pg_stat_statements: calls under-estimation propagation

Daniel Farina escribió: On Thu, Oct 10, 2013 at 7:40 AM, Fujii Masao masao.fu...@gmail.com wrote: In my test, I found that pg_stat_statements.total_time always indicates a zero. I guess that the patch might handle pg_stat_statements.total_time wrongly. +values[i++] =

Re: [HACKERS] pg_stat_statements: calls under-estimation propagation

On Fri, Oct 11, 2013 at 2:49 AM, Alvaro Herrera alvhe...@2ndquadrant.com wrote: Daniel Farina escribió: On Thu, Oct 10, 2013 at 7:40 AM, Fujii Masao masao.fu...@gmail.com wrote: In my test, I found that pg_stat_statements.total_time always indicates a zero. I guess that the patch might

Re: [HACKERS] pg_stat_statements: calls under-estimation propagation

On Thu, Oct 10, 2013 at 10:12 AM, Fujii Masao masao.fu...@gmail.com wrote: On Fri, Oct 11, 2013 at 12:19 AM, Daniel Farina dan...@heroku.com wrote: Probably. The idea is that without those fields it's, to wit, impossible to explain non-monotonic movement in metrics of those queries for

Re: [HACKERS] pg_stat_statements: calls under-estimation propagation

On Thu, Oct 10, 2013 at 10:49 AM, Alvaro Herrera alvhe...@2ndquadrant.com wrote: Daniel Farina escribió: On Thu, Oct 10, 2013 at 7:40 AM, Fujii Masao masao.fu...@gmail.com wrote: In my test, I found that pg_stat_statements.total_time always indicates a zero. I guess that the patch might

Re: [HACKERS] pg_stat_statements: calls under-estimation propagation

Daniel Farina escribió: Given that, perhaps a way to fix this is something like this patch-fragment: { PGSS_TUP_V1_0 = 1, PGSS_TUP_V1_1, - PGSS_TUP_LATEST + PGSS_TUP_V1_2 } pgssTupVersion; +#define PGSS_TUP_LATEST PGSS_TUP_V1_2 This sounds good. I have seen other places

Re: [HACKERS] pg_stat_statements: calls under-estimation propagation

The V7-Patch applied cleanly and I got no issues in my first tests. The change from column session_start to a function seems very reasonable for me. Concernig the usability, I would like to suggest a minor change, that massively increases the usefulness of the patch for beginners, who often

Re: [HACKERS] pg_stat_statements: calls under-estimation propagation

On Thu, Oct 10, 2013 at 1:30 PM, Alvaro Herrera alvhe...@2ndquadrant.com wrote: Just noticed that you changed the timer to struct Instrumentation. Not really sure about that change. Since you seem to be using only the start time and counter, wouldn't it be better to store only those?

Re: [HACKERS] pg_stat_statements: calls under-estimation propagation

On Fri, Oct 4, 2013 at 7:22 AM, Fujii Masao masao.fu...@gmail.com wrote: On Thu, Oct 3, 2013 at 5:11 PM, Sameer Thakur samthaku...@gmail.com wrote: On Wed, Oct 2, 2013 at 6:40 PM, Sameer Thakur samthaku...@gmail.com wrote: Looks pretty good. Do you want to package up the patch with your

Re: [HACKERS] pg_stat_statements: calls under-estimation propagation

Please find the patch attached Thanks for the patch! Here are the review comments: +OUT session_start timestamptz, +OUT introduced timestamptz, The patch exposes these columns in pg_stat_statements view. These should be documented. Yes, will add to documentation. I don't think

  1   2   >