Peter Geoghegan pe...@2ndquadrant.com writes:
Having taken another look at the code, I wonder if we wouldn't have
been better off just fastpathing out of pgss_store in the first call
(in a pair of calls made by a backend as part an execution of some
non-prepared query) iff there is already an
Peter Geoghegan pe...@2ndquadrant.com writes:
On 29 March 2012 21:05, Tom Lane t...@sss.pgh.pa.us wrote:
Barring objections I'll go fix this, and then this patch can be
considered closed except for possible future tweaking of the
sticky-entry decay rule.
Attached patch fixes a bug, and
On 8 April 2012 20:51, Tom Lane t...@sss.pgh.pa.us wrote:
Applied with some cosmetic adjustments.
Thanks.
Having taken another look at the code, I wonder if we wouldn't have
been better off just fastpathing out of pgss_store in the first call
(in a pair of calls made by a backend as part an
On 29 March 2012 21:05, Tom Lane t...@sss.pgh.pa.us wrote:
Barring objections I'll go fix this, and then this patch can be
considered closed except for possible future tweaking of the
sticky-entry decay rule.
Attached patch fixes a bug, and tweaks sticky-entry decay.
The extant code bumps
On Wed, Mar 28, 2012 at 10:39 PM, Tom Lane t...@sss.pgh.pa.us wrote:
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).
This might be a dumb idea,
On 29 March 2012 02:09, Tom Lane t...@sss.pgh.pa.us wrote:
Thanks. I've committed the patch along with the docs, after rather
heavy editorialization.
Thank you.
1. What to do with EXPLAIN, SELECT INTO, etc. We had talked about
tweaking the behavior of statement nesting and some other
Robert Haas robertmh...@gmail.com writes:
On Wed, Mar 28, 2012 at 10:39 PM, Tom Lane t...@sss.pgh.pa.us wrote:
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
On Thu, Mar 29, 2012 at 11:23 AM, Tom Lane t...@sss.pgh.pa.us wrote:
It would make more sense to me to go the other way, that is suppress
creation of a separate entry for the contained optimizable statement.
The stats will still be correctly accumulated into the surrounding
statement (or at
Robert Haas robertmh...@gmail.com writes:
On Thu, Mar 29, 2012 at 11:23 AM, Tom Lane t...@sss.pgh.pa.us wrote:
However, I think there is a solution for that, though it may sound a bit
ugly. Rather than just stacking a flag, let's stack the query source
text pointer for the utility statement.
On Thu, Mar 29, 2012 at 11:42 AM, Tom Lane t...@sss.pgh.pa.us wrote:
Robert Haas robertmh...@gmail.com writes:
On Thu, Mar 29, 2012 at 11:23 AM, Tom Lane t...@sss.pgh.pa.us wrote:
However, I think there is a solution for that, though it may sound a bit
ugly. Rather than just stacking a flag,
Robert Haas robertmh...@gmail.com writes:
What I'm imagining is that instead of just having a global for
nested_level, you'd have a global variable pointing to a linked list.
This is more or less what I have in mind, too, except I do not believe
that a mere boolean flag is sufficient to tell
[ forgot to respond to this bit ]
Robert Haas robertmh...@gmail.com writes:
Another thought is: if we simply treated these as nested queries for
all purposes, would that really be so bad?
That was actually what I suggested first, and now that I look at the
code, that's exactly what's happening
I wrote:
Hm ... I just had a different idea. I need to go look at the code
again, but I believe that in the problematic cases, the post-analyze
hook does not compute a queryId for the optimizable statement. This
means that it will arrive at the executor with queryId zero. What if
we simply
I wrote:
... PREPARE/EXECUTE work a bit funny though: if you have
track = all then you get EXECUTE cycles reported against both the
EXECUTE statement and the underlying PREPARE. This is because when
PREPARE calls parse_analyze_varparams the post-analyze hook doesn't know
that this isn't a
On Thu, Mar 29, 2012 at 4:05 PM, Tom Lane t...@sss.pgh.pa.us wrote:
I wrote:
... PREPARE/EXECUTE work a bit funny though: if you have
track = all then you get EXECUTE cycles reported against both the
EXECUTE statement and the underlying PREPARE. This is because when
PREPARE calls
[ also for the archives' sake ]
On 27 March 2012 22:05, Tom Lane t...@sss.pgh.pa.us wrote:
Well, testing function pointers for null is certainly OK --- note that
all our hook function call sites do that. It's true that testing for
equality to a particular function's name can fail on some
On 27 March 2012 20:26, Tom Lane t...@sss.pgh.pa.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
Peter Geoghegan pe...@2ndquadrant.com writes:
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
On 28 March 2012 15:25, Tom Lane t...@sss.pgh.pa.us wrote:
That's been an issue right along for cases such as EXPLAIN and EXECUTE,
I believe.
Possible, since I didn't have test coverage for either of those 2 commands.
Perhaps the right thing is to consider such executor calls
as nested
A couple other issues about this patch ...
Is there any actual benefit in providing the
pg_stat_statements.string_key GUC? It looks to me more like something
that was thrown in because it was easy than because anybody would want
it. I'd just as soon leave it out and avoid the incremental API
On 28 March 2012 15:57, Tom Lane t...@sss.pgh.pa.us wrote:
Is there any actual benefit in providing the
pg_stat_statements.string_key GUC? It looks to me more like something
that was thrown in because it was easy than because anybody would want
it. I'd just as soon leave it out and avoid the
Peter Geoghegan pe...@2ndquadrant.com writes:
On 28 March 2012 15:57, Tom Lane t...@sss.pgh.pa.us wrote:
Is there any actual benefit in providing the
pg_stat_statements.string_key GUC? It looks to me more like something
that was thrown in because it was easy than because anybody would want
On 29 March 2012 00:14, Tom Lane t...@sss.pgh.pa.us wrote:
I'm planning to commit the patch with a USAGE_NON_EXEC_STICK value
of 3.0, which is the largest value that stays below 10% wastage.
We can twiddle that logic later, so if you want to experiment with an
alternate decay rule, feel free.
Peter Geoghegan pe...@2ndquadrant.com writes:
doc-patch is attached. I'm not sure if I got the balance right - it
may be on the verbose side.
Thanks. I've committed the patch along with the docs, after rather
heavy editorialization.
There remain some loose ends that should be worked on but
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
Peter Geoghegan pe...@2ndquadrant.com writes:
On 22 March 2012 17:19, Tom Lane t...@sss.pgh.pa.us wrote:
Either way, I think we'd be a lot better advised to define a single
hook post_parse_analysis_hook and make the core code responsible
for calling it at the appropriate places, rather than
Peter Geoghegan pe...@2ndquadrant.com writes:
I've attached a patch with the required modifications.
I've committed the core-backend parts of this, just to get them out of
the way. Have yet to look at the pg_stat_statements code itself.
I restored the location field to the ParamCoerceHook
On 27 March 2012 20:26, Tom Lane t...@sss.pgh.pa.us wrote:
I've committed the core-backend parts of this, just to get them out of
the way. Have yet to look at the pg_stat_statements code itself.
Thanks. I'm glad that we have that out of the way.
I ended up choosing not to apply that bit. I
[ just for the archives' sake ]
Peter Geoghegan pe...@2ndquadrant.com writes:
On 27 March 2012 18:15, Tom Lane t...@sss.pgh.pa.us wrote:
Now, if what it wants to know about is the parameterization status
of the query, things aren't ideal because most of the info is hidden
in parse-callback
Peter Geoghegan pe...@2ndquadrant.com writes:
[ pg_stat_statements_norm_2012_02_29.patch ]
I started to look at this patch (just the core-code modifications so
far). There are some things that seem not terribly well thought out:
* It doesn't look to me like it will behave very sanely with
On 22 March 2012 17:19, Tom Lane t...@sss.pgh.pa.us wrote:
I'm inclined to think that the most useful behavior is to teach the
rewriter to copy queryId from the original query into all the Queries
generated by rewrite. Then, all rules fired by a source query would
be lumped into that query
Peter Geoghegan pe...@2ndquadrant.com writes:
Since you haven't mentioned the removal of parse-analysis Const
location alterations, I take it that you do not object to that, which
is something I'm glad of.
I remain un-thrilled about it, but apparently nobody else cares, so
I'll yield the
On 22 March 2012 19:07, Tom Lane t...@sss.pgh.pa.us wrote:
Will you adjust the patch for the other issues?
Sure. I'll take a look at it now.
--
Peter Geoghegan http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training and Services
--
Sent via pgsql-hackers mailing list
On Mon, Mar 19, 2012 at 08:48:07PM +, Peter Geoghegan wrote:
On 19 March 2012 19:55, Peter Eisentraut pete...@gmx.net wrote:
If someone wanted to bite the bullet and do the work, I think we could
move to a Perl/TAP-based test suite (not pgTAP, but Perl and some fairly
standard Test::*
On mån, 2012-03-19 at 02:35 +, Peter Geoghegan wrote:
I see your point of view. I suppose I can privately hold onto the test
suite, since it might prove useful again.
I would still like to have those tests checked in, but not run by
default, in case someone wants to hack on this particular
On mån, 2012-03-19 at 08:59 +, Greg Stark wrote:
The other problem with this approach is that it's hard to keep a huge
test suite 100% clean. Changes inevitably introduce behaviour changes
that cause some of the tests to fail.
I think we are used to that because of the way pg_regress
On 19 March 2012 19:55, Peter Eisentraut pete...@gmx.net wrote:
If someone wanted to bite the bullet and do the work, I think we could
move to a Perl/TAP-based test suite (not pgTAP, but Perl and some fairly
standard Test::* modules) and reduce that useless reformatting work and
test more
On Sun, Mar 18, 2012 at 7:35 PM, Peter Geoghegan pe...@2ndquadrant.com wrote:
On 19 March 2012 01:50, Tom Lane t...@sss.pgh.pa.us wrote:
I am *not* a fan of regression tests that try to microscopically test
every feature in the system.
I see your point of view. I suppose I can privately hold
Peter Geoghegan pe...@2ndquadrant.com writes:
Is there anything that I could be doing to help bring this patch
closer to a committable state?
Sorry, I've not actually looked at that patch yet. I felt I should
push on Andres' CTAS patch first, since that's blocking progress on
the command
On 18 March 2012 16:13, Tom Lane t...@sss.pgh.pa.us wrote:
Is there a really strong reason why adequate regression testing isn't
possible in a plain-vanilla pg_regress script? A quick look at the
script says that it's just doing some SQL commands and then checking the
results of queries on
On 03/18/2012 06:12 PM, Peter Geoghegan wrote:
On 18 March 2012 16:13, Tom Lanet...@sss.pgh.pa.us wrote:
Is there a really strong reason why adequate regression testing isn't
possible in a plain-vanilla pg_regress script? A quick look at the
script says that it's just doing some SQL
On 18 March 2012 22:46, Andrew Dunstan and...@dunslane.net wrote:
If you want to generate the tests using some tool, then use whatever works
for you, be it Python or Perl or Valgol, but ideally what is committed (and
this what should be in your patch) will be the SQL output of that, not the
On 03/18/2012 07:46 PM, Peter Geoghegan wrote:
On 18 March 2012 22:46, Andrew Dunstanand...@dunslane.net wrote:
If you want to generate the tests using some tool, then use whatever works
for you, be it Python or Perl or Valgol, but ideally what is committed (and
this what should be in your
On 19 March 2012 00:10, Andrew Dunstan and...@dunslane.net wrote:
If your tests are that voluminous then maybe they are not what we're looking
for anyway. As Tom noted:
IMO the objective of a regression test is not to memorialize every single
case the code author thought about during
Peter Geoghegan pe...@2ndquadrant.com writes:
On 19 March 2012 00:10, Andrew Dunstan and...@dunslane.net wrote:
Why exactly does this feature need particularly to have script-driven
regression test generation when others don't?
It's not that it needs it, so much as that it is possible to
On 19 March 2012 01:50, Tom Lane t...@sss.pgh.pa.us wrote:
I am *not* a fan of regression tests that try to microscopically test
every feature in the system.
I see your point of view. I suppose I can privately hold onto the test
suite, since it might prove useful again.
I will work on a
Is there anything that I could be doing to help bring this patch
closer to a committable state? I'm thinking of the tests in particular
- do you suppose it's acceptable to commit them more or less as-is?
The standard for testing contrib modules seems to be a bit different,
as there is a number of
On 2 March 2012 20:10, Tom Lane t...@sss.pgh.pa.us wrote:
I do intend to take this one up in due course
I probably should have exposed the query_id directly in the
pg_stat_statements view, perhaps as query_hash. The idea of that
would be to advertise the potential non-uniqueness of the value - a
Peter Geoghegan pe...@2ndquadrant.com writes:
I probably should have exposed the query_id directly in the
pg_stat_statements view, perhaps as query_hash.
FWIW, I think that's a pretty bad idea; the hash seems to me to be
strictly an internal matter. Given the sponginess of its definition
I
On Sat, Mar 3, 2012 at 12:01 AM, Robert Haas robertmh...@gmail.com wrote:
On Fri, Mar 2, 2012 at 4:56 PM, Simon Riggs si...@2ndquadrant.com wrote:
Checksums patch isn't sucking much attention at all but admittedly
there are some people opposed to the patch that want to draw out the
On 03/05/2012 05:12 AM, Simon Riggs wrote:
On Sat, Mar 3, 2012 at 12:01 AM, Robert Haasrobertmh...@gmail.com wrote:
On Fri, Mar 2, 2012 at 4:56 PM, Simon Riggssi...@2ndquadrant.com wrote:
Checksums patch isn't sucking much attention at all but admittedly
there are some people opposed to
It would probably be prudent to concentrate on getting the core
infrastructure committed first. That way, we at least know that if
this doesn't get into 9.2, we can work on getting it into 9.3 knowing
that once committed, people won't have to wait over a year at the very
I don't see why we
Josh Berkus j...@agliodbs.com writes:
It would probably be prudent to concentrate on getting the core
infrastructure committed first. That way, we at least know that if
this doesn't get into 9.2, we can work on getting it into 9.3 knowing
that once committed, people won't have to wait over a
On Fri, Mar 2, 2012 at 12:48 PM, Tom Lane t...@sss.pgh.pa.us wrote:
Josh Berkus j...@agliodbs.com writes:
It would probably be prudent to concentrate on getting the core
infrastructure committed first. That way, we at least know that if
this doesn't get into 9.2, we can work on getting it into
On Fri, Mar 2, 2012 at 5:48 PM, Tom Lane t...@sss.pgh.pa.us wrote:
Josh Berkus j...@agliodbs.com writes:
It would probably be prudent to concentrate on getting the core
infrastructure committed first. That way, we at least know that if
this doesn't get into 9.2, we can work on getting it into
We'll get to it in due time. In case you haven't noticed, there's a lot
of stuff in this commitfest. And I don't follow the logic that says
that because Simon is trying to push through a not-ready-for-commit
patch we should drop our standards for other patches.
What I'm pointing out is
Robert Haas robertmh...@gmail.com writes:
On Fri, Mar 2, 2012 at 12:48 PM, Tom Lane t...@sss.pgh.pa.us wrote:
We'll get to it in due time. In case you haven't noticed, there's a lot
of stuff in this commitfest. And I don't follow the logic that says
that because Simon is trying to push
Josh Berkus j...@agliodbs.com writes:
This is exactly why I'm not keen on checksums for 9.2. We've reached
the point where the attention on the checksum patch is pushing aside
other patches which are more ready and have had more work.
IMO the reason why it's sucking so much attention is
On Fri, Mar 2, 2012 at 8:13 PM, Tom Lane t...@sss.pgh.pa.us wrote:
Josh Berkus j...@agliodbs.com writes:
This is exactly why I'm not keen on checksums for 9.2. We've reached
the point where the attention on the checksum patch is pushing aside
other patches which are more ready and have had
On Fri, Mar 2, 2012 at 4:56 PM, Simon Riggs si...@2ndquadrant.com wrote:
Checksums patch isn't sucking much attention at all but admittedly
there are some people opposed to the patch that want to draw out the
conversation until the patch is rejected,
Wow. Sounds like a really shitty thing for
On 1 March 2012 00:48, Alvaro Herrera alvhe...@commandprompt.com wrote:
I'm curious about the LeafNode stuff. Is this something that could be
done by expression_tree_walker? I'm not completely familiar with it so
maybe there's some showstopper such as some node tags not being
supported, or
On Thu, Mar 1, 2012 at 1:27 PM, Peter Geoghegan pe...@2ndquadrant.com wrote:
My expectation is that this feature will make life a lot
easier for a lot of Postgres users.
Yes. It's hard to overstate the apparent utility of this feature in
the general category of visibility and profiling.
--
On 3/1/12 1:57 PM, Daniel Farina wrote:
On Thu, Mar 1, 2012 at 1:27 PM, Peter Geoghegan pe...@2ndquadrant.com wrote:
My expectation is that this feature will make life a lot
easier for a lot of Postgres users.
Yes. It's hard to overstate the apparent utility of this feature in
the general
On 1 March 2012 22:09, Josh Berkus j...@agliodbs.com wrote:
On 3/1/12 1:57 PM, Daniel Farina wrote:
On Thu, Mar 1, 2012 at 1:27 PM, Peter Geoghegan pe...@2ndquadrant.com
wrote:
My expectation is that this feature will make life a lot
easier for a lot of Postgres users.
Yes. It's hard to
On Mon, Feb 27, 2012 at 4:26 AM, Peter Geoghegan pe...@2ndquadrant.com wrote:
This does not appear to have any user-visible effect on caret position
for all variations in coercion syntax, while giving me everything that
I need. I had assumed that we were relying on things being this way,
but
I'm curious about the LeafNode stuff. Is this something that could be
done by expression_tree_walker? I'm not completely familiar with it so
maybe there's some showstopper such as some node tags not being
supported, or maybe it just doesn't help. But I'm curious.
--
Álvaro Herrera
On 27 February 2012 06:23, Tom Lane t...@sss.pgh.pa.us wrote:
I think that what Peter is on about in
http://archives.postgresql.org/pgsql-hackers/2012-02/msg01152.php
is the question of what location to use for the *result* of
'literal string'::typename, assuming that the type's input function
On Fri, Feb 24, 2012 at 9:43 AM, Peter Geoghegan pe...@2ndquadrant.com wrote:
Tom's point example does not seem to be problematic to me - the cast
*should* blame the 42 const token, as the cast doesn't work as a
result of its representation, which is in point of fact why the core
system blames
Robert Haas robertmh...@gmail.com writes:
I think I agree Tom's position upthread: blaming the coercion seems to
me to make more sense. But if that's what we're trying to do, then
why does parse_coerce() say this?
/*
* Set up to point at the constant's text if the input
On 21 February 2012 01:48, Tom Lane t...@sss.pgh.pa.us wrote:
you're proposing to move the error pointer to the 42, and that does
not seem like an improvement, especially not if it only happens when the
cast subject is a simple constant rather than an expression.
2008's commit
On 21 February 2012 01:48, Tom Lane t...@sss.pgh.pa.us wrote:
you're proposing to move the error pointer to the 42, and that does
not seem like an improvement, especially not if it only happens when the
cast subject is a simple constant rather than an expression.
I'm not actually proposing
Peter Geoghegan pe...@2ndquadrant.com writes:
Here is the single, hacky change I've made just for now to the core
parser to quickly see if it all works as expected:
*** transformTypeCast(ParseState *pstate, Ty
*** 2108,2113
--- 2108,2116
if (location 0)
Peter Geoghegan pe...@2ndquadrant.com writes:
Another look around shows that the CoerceToDomain struct takes its
location from the new Const location in turn, so my dirty little hack
will break the location of the CoerceToDomain struct, giving an
arguably incorrect caret position in some error
73 matches
Mail list logo
