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

On Mon, Dec 5, 2011 at 19:31, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)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 such a control boolean in the first place?

It's needed for correctness with PL/pgSQL simple expressions.

This seems a bit of a kludge, but I considered it the "least bad"
solution. Here's what I added to planner.c standard_planner():

/*
* glob->isSimple is a hint to eval_const_expressions() and PL/pgSQL that
* this statement is potentially a simple expression -- it contains no
* table references, no subqueries and no join clauses.
*
* We need this here because this prevents insertion of CacheExpr, which
* would break simple expressions in PL/pgSQL. Such queries wouldn't
* benefit from constant caching anyway.
*
* The actual definition of a simple statement is more strict, but we
* don't want to spend that checking overhead here.
*
* Caveat: Queries with set-returning functions in SELECT list could
* still potentially benefit from caching, but we don't check that now.
*/
glob->isSimple = (parse->commandType == CMD_SELECT &&
parse->jointree->fromlist == NIL &&
parse->hasSubLinks == FALSE &&
parse->cteList == NIL);

----

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).

Currently I'm also disabling CacheExpr nodes in
estimate_expression_value() since we know for a fact that the planner
only evaluates it once. But that probably doesn't make much of a
difference.

Regards,
Marti