On Tue, Jan 24, 2023 at 10:07 AM David Rowley <dgrowle...@gmail.com> wrote:

> I'm surprised to see that it's only Memoize that ever makes use of
> lateral_vars. I'd need a bit more time to process your patch, but one
> additional thought I had was that I wonder if the following code is
> still needed in nodeMemoize.c
>
> if (bms_nonempty_difference(outerPlan->chgParam, node->keyparamids))
>     cache_purge_all(node);
>
> Ideally, that would be an Assert failure, but possibly we should
> probably still call cache_purge_all(node) after Assert(false) so that
> at least we'd not start returning wrong results if we've happened to
> miss other cache keys. I thought maybe something like:


Hmm, I think this code is still needed because the parameter contained
in the subplan below a Memoize node may come from parent plan, as in the
test query added in 411137a42.

EXPLAIN (COSTS OFF)
SELECT unique1 FROM tenk1 t0
WHERE unique1 < 3
  AND EXISTS (
    SELECT 1 FROM tenk1 t1
    INNER JOIN tenk1 t2 ON t1.unique1 = t2.hundred
    WHERE t0.ten = t1.twenty AND t0.two <> t2.four OFFSET 0);
                           QUERY PLAN
----------------------------------------------------------------
 Index Scan using tenk1_unique1 on tenk1 t0
   Index Cond: (unique1 < 3)
   Filter: (SubPlan 1)
   SubPlan 1
     ->  Nested Loop
           ->  Index Scan using tenk1_hundred on tenk1 t2
                 Filter: (t0.two <> four)
           ->  Memoize
                 Cache Key: t2.hundred
                 Cache Mode: logical
                 ->  Index Scan using tenk1_unique1 on tenk1 t1
                       Index Cond: (unique1 = t2.hundred)
                       Filter: (t0.ten = twenty)
(13 rows)

Currently we don't have a way to add Params of uplevel vars to Memoize
cache keys.  So I think we still need to call cache_purge_all() each
time uplevel Params change.

Thanks
Richard

Reply via email to