Peter Geoghegan 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 entry in the hasht
On 8 April 2012 20:51, Tom Lane 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 execution of some
non
Peter Geoghegan writes:
> On 29 March 2012 21:05, Tom Lane 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.
Applied w
On 29 March 2012 21:05, Tom Lane 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 usage (though not c
On Thu, Mar 29, 2012 at 4:05 PM, Tom Lane 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 parse_analyze_varpara
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
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 si
[ forgot to respond to this bit ]
Robert Haas 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 right now. However
Robert Haas 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 the difference between
On Thu, Mar 29, 2012 at 11:42 AM, Tom Lane wrote:
> Robert Haas writes:
>> On Thu, Mar 29, 2012 at 11:23 AM, Tom Lane 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
Robert Haas writes:
> On Thu, Mar 29, 2012 at 11:23 AM, Tom Lane 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. Then in the executor hooks,
>> i
On Thu, Mar 29, 2012 at 11:23 AM, Tom Lane 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 least, if they ar
Robert Haas writes:
> On Wed, Mar 28, 2012 at 10:39 PM, Tom Lane 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 d
On 29 March 2012 02:09, Tom Lane 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 possibilities.
> I t
On Wed, Mar 28, 2012 at 10:39 PM, Tom Lane 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, but for a quick ha
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 run
Peter Geoghegan 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 didn't seem
like commi
On 29 March 2012 00:14, Tom Lane 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.
I think I may
Peter Geoghegan writes:
> On 28 March 2012 15:57, Tom Lane 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
On 28 March 2012 15:57, Tom Lane 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 incremental
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
c
On 28 March 2012 15:25, Tom Lane 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 statements --- that i
Peter Geoghegan 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 CreateTableAsStmt".
On 27 March 2012 20:26, Tom Lane 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, "Restructu
[ also for the archives' sake ]
On 27 March 2012 22:05, Tom Lane 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 platforms
> because
[ just for the archives' sake ]
Peter Geoghegan writes:
> On 27 March 2012 18:15, Tom Lane 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 fields that aren't of globally expo
On 27 March 2012 20:26, Tom Lane 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 remain of the op
Peter Geoghegan 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 signature, but
> the re
Peter Geoghegan writes:
>> On 22 March 2012 17:19, Tom Lane 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 supposing that
>>> the contri
On 22 March 2012 19:07, Tom Lane 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 (pgsql-hackers@po
Peter Geoghegan 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 point. (I do however ob
On 22 March 2012 17:19, Tom Lane 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 for tracking purp
Peter Geoghegan 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 rules.
The patch doesn't
On Mon, Mar 19, 2012 at 08:48:07PM +, Peter Geoghegan wrote:
> On 19 March 2012 19:55, Peter Eisentraut 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) a
On Sun, Mar 18, 2012 at 7:35 PM, Peter Geoghegan wrote:
> On 19 March 2012 01:50, Tom Lane 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 m
On 19 March 2012 19:55, Peter Eisentraut 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 interesting thin
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 wor
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 19 March 2012 01:50, Tom Lane 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 pg_regress based approa
Peter Geoghegan writes:
> On 19 March 2012 00:10, Andrew Dunstan 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 provide
> coverage for much of the code
On 19 March 2012 00:10, Andrew Dunstan 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 development. ISTM
On 03/18/2012 07:46 PM, Peter Geoghegan wrote:
On 18 March 2012 22:46, Andrew Dunstan 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 SQ
On 18 March 2012 22:46, Andrew Dunstan 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
> generator plus inp
On 03/18/2012 06:12 PM, Peter Geoghegan wrote:
On 18 March 2012 16:13, Tom Lane 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
On 18 March 2012 16:13, Tom Lane 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 the pg_stat_state
Peter Geoghegan 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 triggers patch.
> I'm th
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
Peter Geoghegan 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 don't really want it
On 2 March 2012 20:10, Tom Lane 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
collision is *e
On 03/05/2012 05:12 AM, Simon Riggs wrote:
On Sat, Mar 3, 2012 at 12:01 AM, Robert Haas wrote:
On Fri, Mar 2, 2012 at 4:56 PM, Simon Riggs 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
conversa
On Sat, Mar 3, 2012 at 12:01 AM, Robert Haas wrote:
> On Fri, Mar 2, 2012 at 4:56 PM, Simon Riggs 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,
>
>
On Fri, Mar 2, 2012 at 4:56 PM, Simon Riggs 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 those people to do
On Fri, Mar 2, 2012 at 8:13 PM, Tom Lane wrote:
> Josh Berkus 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 w
Josh Berkus 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 precisely that it's
Robert Haas writes:
> On Fri, Mar 2, 2012 at 12:48 PM, Tom Lane 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 through a not-ready-for-commit
>> patc
> 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
On Fri, Mar 2, 2012 at 5:48 PM, Tom Lane wrote:
> Josh Berkus 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
On Fri, Mar 2, 2012 at 12:48 PM, Tom Lane wrote:
> Josh Berkus 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
Josh Berkus 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 year at the
> 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 w
On 1 March 2012 22:09, Josh Berkus wrote:
> On 3/1/12 1:57 PM, Daniel Farina wrote:
>> On Thu, Mar 1, 2012 at 1:27 PM, Peter Geoghegan
>> 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 ut
On 3/1/12 1:57 PM, Daniel Farina wrote:
> On Thu, Mar 1, 2012 at 1:27 PM, Peter Geoghegan 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 vi
On Thu, Mar 1, 2012 at 1:27 PM, Peter Geoghegan 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.
--
fdr
--
Sent via pgs
On 1 March 2012 00:48, Alvaro Herrera 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 maybe it just doesn't he
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
The Postg
On Mon, Feb 27, 2012 at 4:26 AM, Peter Geoghegan 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 apparently this is
On 27 February 2012 06:23, Tom Lane 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
> doesn't comp
Robert Haas 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 routine throws
>
On Fri, Feb 24, 2012 at 9:43 AM, Peter Geoghegan 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 the Const node an
On 21 February 2012 01:48, Tom Lane 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 a2794623d292f7bbfe3134d1407281
On 21 February 2012 01:48, Tom Lane 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 that though. What
Peter Geoghegan 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 messages. It woul
Peter Geoghegan 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)
> locatio
73 matches
Mail list logo
