Re: bug: query returns different result with and without memoization.

On Mon, Jul 27, 2026 at 9:46 PM David Rowley <dgrowleyml(at)gmail(dot)com> wrote:
> For reference, I was experimenting with the attached. What do you think?

I looked over your patch, but I'm afraid it's not correct.

IIUC, your patch inspects innerrel->baserestrictinfo and only collects
the outer arg of a two-arg OpExpr.

First, this can pick the wrong arg. If the Param isn't a whole side
of the qual, we end up adding a bogus cache key while still missing
the Param itself. Consider the qual "t1.twenty - t0.ten = 0" in this
query:

SELECT sum(c) FROM (
SELECT t0.unique1,
(SELECT count(*) FROM tenk1 t2 JOIN tenk1 t1
ON t1.unique1 = t2.hundred + t0.ten
WHERE t1.twenty - t0.ten = 0) AS c
FROM tenk1 t0 WHERE t0.unique1 < 200) s;

With your patch I get:

-> Memoize
Cache Key: (t2.hundred + t0.ten), 0
Cache Mode: logical
-> Index Scan using tenk1_unique1 on tenk1 t1
Index Cond: (unique1 = (t2.hundred + t0.ten))
Filter: ((twenty - t0.ten) = 0)

That is, the constant 0 becomes a cache key, while t0.ten is still
missing from the keys.

And running this query I get wrong result:

sum
--------
148000

where the correct result is 100000.

Second, I'm not convinced that base quals are the only place such a
Param can appear. It can also appear on the inner side of a join
clause, as in "t1.hundred + t0.ten = t2.hundred", where only the outer
side becomes a cache key. It could also be in innerrel->reltarget, or
inside a subquery RTE that wasn't pulled up, and no amount of base
qual scanning will reach those.

That is what worries me most about this approach: it requires us to
enumerate every place a Param might be used, and if we ever miss one,
we are back to the same kind of wrong results.

Besides correctness, I'm a bit concerned that returning false discards
the Memoize path entirely, and that a qual only has to contain a Param
to reach that test. Taking the same query with a plain join clause
and varying only the qual on t1:

SELECT sum(c) FROM (
SELECT (SELECT count(*) FROM tenk1 t1
INNER JOIN tenk1 t2 ON t1.unique1 = t2.hundred
WHERE <qual>) AS c
FROM tenk1 t0 WHERE t0.unique1 < 2) s;

none of these can use Memoize any more with your patch, though none of
them is broken today:

WHERE t1.twenty = t1.four + t0.ten -- sides don't match
WHERE t1.twenty IS DISTINCT FROM t0.ten -- not an OpExpr
WHERE t1.twenty = t0.ten OR t1.four = t0.two -- not an OpExpr

along with any Param whose type has no hash opclass. Today each of
those gets a working Memoize node that simply flushes the cache when
the Param changes. So I'm concerned about the plan regression in
cases that never had a problem.

So I'd like to propose the attached patch. Rather than looking for
every place a Param might be used, it requires that every PARAM_EXEC
Param appearing in a cache key expression is also a cache key in its
own right. Then matching the keys implies matching the Param, and the
set we have to examine is bounded, because it's just the key list we
already built.

The patch then also makes cache keys of the Params used outside the
cache keys: in the relation's base quals, its targetlist, and on the
inner side of the join clauses. It's what stops the cache being
purged on every invocation of a correlated subplan. It's only an
optimization, so it's ok if we miss a place a Param might be used.

- Richard