Re: Caching for stable expressions with constant arguments v6

On Mon, Jan 16, 2012 at 19:06, Marti Raudsepp <marti(at)juffo(dot)org> wrote:
> Here's v6 of my expression caching patch. The only change in v6 is
> added expression cost estimation in costsize.c.

Ok, I found another omission in my patch. Up until v4, the behavior of
estimate_expression_value() was to never insert CacheExpr nodes, and
always strip them when found. In v5, I ripped out all the conditional
CacheExpr insertion and stripping so expression trees would always be
normalized the same way.

This had an unintended consequence; operator selectivity estimation
functions aren't prepared to deal with CacheExpr nodes. E.g. STABLE
function calls and expressions with Params are constant-folded in
estimation mode, but the constant would still remain as a child of a
CacheExpr node.

The solutions I considered:
1. Selectivity functions and other estimation callers could be changed
to strip CacheExpr themself. This doesn't seem very attractive since
there are quite a lot of them and it seems like duplication of code.

2. Strip CacheExpr nodes in the estimation pass. This has a potential
hazard that estimate_expression_value() is no longer idempotent; a
second pass over the same expr tree might re-insert CacheExpr to some
places. However, these cases are unlikely to be problematic for
selectivity estimation anyway -- currently selectivity estimation (and
other callers) can only deal with trees that get folded to a single
Const, and those don't get cached.

3. Strip CacheExprs *and* restrict CacheExpr insertion in estimation
mode (like v4 patch). This would also add a larger amount of code to
the patch, but it would keep estimate_expression_value() idempotent.

----

The attached patch implements solution #2 as it's the least amount of
code and the downside doesn't seem problematic (as explained above). I
also rebased the patch to current Postgres master (no conflicts, just
some fuzz). Note that the create_index test fails in master, this
problem isn't introduced by my patch.

As always, this work is also available from my Github "cache" branch:
https://github.com/intgr/postgres/commits/cache

Regards,
Marti