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

On Sun, Dec 4, 2011 at 22:53, Heikki Linnakangas
<heikki.linnakangas@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