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 separate pass would be > more expensive. And there are callers of eval_const_expressions() that have > no use for the caching, like process_implied_equality().
I had an idea how to implement this approach with minimal performance impact. It might well be a dumb idea, but I wanted to get it out there. 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. Based on this information, ExecInitExpr (which is pretty simple for now) could decide where to inject CacheExprState nodes. It's easier to make that decision there, since by that stage the cachability of the parent node with each argument is already known. (Currently I have to stick all cachable arguments in a temporary list until I've determined whether all arguments are cachable, or just some of them are) This would decouple expression caching from the planner entirely; CacheExpr wouldn't exist anymore. AFAICT this would also let us get rid of contain_mutable_functions_walker() and possibly others. Regards, Marti -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
