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

On Mon, Dec 5, 2011 at 19:31, Tom Lane <tgl@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 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