Re: [HACKERS] [PATCH] Caching for stable expressions with constant arguments v3

2011-12-17 Thread Marti Raudsepp
On Fri, Dec 16, 2011 at 21:32, Jaime Casanova ja...@2ndquadrant.com wrote: Actually i tried some benchmarks with the original version of the patch and saw some regression with normal pgbench runs, but it wasn't much... so i was trying to found out some queries that show benefit now that we

Re: [HACKERS] [PATCH] Caching for stable expressions with constant arguments v3

2011-12-16 Thread Marti Raudsepp
On Fri, Dec 16, 2011 at 09:13, Greg Smith g...@2ndquadrant.com wrote: I think what would be helpful here next is a self-contained benchmarking script. Alright, a simple script is attached. One of the first questions I have is whether the performance changed between there and your v5. Not

Re: [HACKERS] [PATCH] Caching for stable expressions with constant arguments v3

2011-12-16 Thread Greg Smith
That looks easy enough to try out now, thanks for the script. Jaime was interested in trying this out, and I hope he still finds time to do that soon. Now that things have settled down and it's obvious how to start testing, that should be a project he can take on. I don't think this is

Re: [HACKERS] [PATCH] Caching for stable expressions with constant arguments v3

2011-12-16 Thread Marti Raudsepp
On Fri, Dec 16, 2011 at 18:08, Greg Smith g...@2ndquadrant.com wrote: I don't think this is going to reach ready to commit in the next few days though, so I'm going to mark it as returned for this CommitFest. Fair enough, I just hope this doesn't get dragged into the next commitfest without

Re: [HACKERS] [PATCH] Caching for stable expressions with constant arguments v3

2011-12-16 Thread Jaime Casanova
On Fri, Dec 16, 2011 at 11:08 AM, Greg Smith g...@2ndquadrant.com wrote: That looks easy enough to try out now, thanks for the script. Jaime was interested in trying this out, and I hope he still finds time to do that soon. Actually i tried some benchmarks with the original version of the

Re: [HACKERS] [PATCH] Caching for stable expressions with constant arguments v3

2011-12-15 Thread Greg Smith
I think what would be helpful here next is a self-contained benchmarking script. You posted an outline of what you did for the original version at http://archives.postgresql.org/message-id/cabrt9rc-1wgxzc_z5mwkdk70fgy2drx3slxzdp4vobkukpz...@mail.gmail.com It doesn't sound like it would be

Re: [HACKERS] [PATCH] Caching for stable expressions with constant arguments v3

2011-12-12 Thread Marti Raudsepp
On Sun, Dec 4, 2011 at 22:53, Heikki Linnakangas heikki.linnakan...@enterprisedb.com wrote: I wonder if it would be better to add the CacheExpr nodes to the tree as a separate pass, instead of shoehorning it into eval_const_expressions? I think would be more readable that way, even though a

Re: [HACKERS] [PATCH] Caching for stable expressions with constant arguments v3

2011-12-12 Thread Tom Lane
Marti Raudsepp ma...@juffo.org writes: The 'struct Expr' type could have another attribute that stores whether its sub-expressions contain stable/volatile functions, and whether it only contains of constant arguments. This attribute would be filled in for every Expr by eval_const_expressions.

Re: [HACKERS] [PATCH] Caching for stable expressions with constant arguments v3

2011-12-10 Thread Greg Smith
On 12/07/2011 04:58 PM, Marti Raudsepp wrote: PS: I forgot to mention that 2 test cases covering the two above query types are deliberately left failing in the v4-wip patch. It's not completely clear what happens next with this. Are you hoping code churn here has calmed down enough for

Re: [HACKERS] [PATCH] Caching for stable expressions with constant arguments v3

2011-12-10 Thread Marti Raudsepp
On Sat, Dec 10, 2011 at 16:50, Greg Smith g...@2ndquadrant.com wrote: Or should we wait for a new update based on the feedback that Heikki and Tom have already provided first, perhaps one that proposes fixes for these two test cases? Yes, I will post and updated and rebased version tomorrow.

Re: [HACKERS] [PATCH] Caching for stable expressions with constant arguments v3

2011-12-07 Thread Marti Raudsepp
On Wed, Dec 7, 2011 at 00:29, Marti Raudsepp ma...@juffo.org wrote: ExecInitExpr enables the cache when its 'PlanState *parent' attribute isn't NULL [...] On the other hand, a few places lose caching support this way since they don't go through the planner: * Column defaults in a COPY FROM

Re: [HACKERS] [PATCH] Caching for stable expressions with constant arguments v3

2011-12-07 Thread Tom Lane
Marti Raudsepp ma...@juffo.org writes: Let me rephrase that as a question: Does it seem worthwhile to add a new argument to ExecInitExpr to handle those two cases? Possibly. Another way would be to keep its API as-is and introduce a different function name for the other behavior. I would

Re: [HACKERS] [PATCH] Caching for stable expressions with constant arguments v3

2011-12-05 Thread Marti Raudsepp
On Sun, Dec 4, 2011 at 22:53, Heikki Linnakangas heikki.linnakan...@enterprisedb.com wrote: This seems to have bitrotted, thanks to the recent refactoring in eval_const_expressions(). Luckily the conflicts are mostly whitespace changes, so shouldn't be hard to fix. I'll try to come up with an

Re: [HACKERS] [PATCH] Caching for stable expressions with constant arguments v3

2011-12-05 Thread Tom Lane
Marti Raudsepp ma...@juffo.org writes: On Sun, Dec 4, 2011 at 22:53, Heikki Linnakangas heikki.linnakan...@enterprisedb.com wrote: This comment in RelationGetExpressions() also worries me: [...] Do the injected CacheExprs screw up that equality? Or the constraint exclusion logic in

Re: [HACKERS] [PATCH] Caching for stable expressions with constant arguments v3

2011-12-05 Thread Marti Raudsepp
On Mon, Dec 5, 2011 at 19:31, Tom Lane t...@sss.pgh.pa.us wrote: I think if you have some call sites inject CacheExprs and others not, it will get more difficult to match up expressions that should be considered equal.  On the whole this seems like a bad idea.  What is the reason for having

Re: [HACKERS] [PATCH] Caching for stable expressions with constant arguments v3

2011-12-05 Thread Heikki Linnakangas
On 05.12.2011 20:53, Marti Raudsepp wrote: I considered stripping CacheExpr nodes later in PL/pgSQL, but I can't remember right now why I rejected that approach (sorry, it's been 2 months). Yet another idea would be to leave the CacheExprs there, but provide a way to reset the caches.

Re: [HACKERS] [PATCH] Caching for stable expressions with constant arguments v3

2011-12-05 Thread Tom Lane
Heikki Linnakangas heikki.linnakan...@enterprisedb.com writes: On 05.12.2011 20:53, Marti Raudsepp wrote: I considered stripping CacheExpr nodes later in PL/pgSQL, but I can't remember right now why I rejected that approach (sorry, it's been 2 months). Yet another idea would be to leave the

Re: [HACKERS] [PATCH] Caching for stable expressions with constant arguments v3

2011-12-05 Thread Heikki Linnakangas
On 05.12.2011 21:36, Tom Lane wrote: Heikki Linnakangasheikki.linnakan...@enterprisedb.com writes: Or pass a flag to ExecInitExpr() to skip through the CacheExprs. Not sure what you mean by that --- are you imagining that the ExprState tree would have structure not matching the Expr tree?

Re: [HACKERS] [PATCH] Caching for stable expressions with constant arguments v3

2011-12-04 Thread Jaime Casanova
On Sat, Sep 24, 2011 at 9:09 PM, Marti Raudsepp ma...@juffo.org wrote: Hi list! This is the third version of my CacheExpr patch. i wanted to try this, but it no longer applies in head... specially because of the change of IF's for a switch in src/backend/optimizer/util/clauses.c it also needs