From 5d4c5cb5a6d59691ee61c5280647483b23e17c6c Mon Sep 17 00:00:00 2001 From: Richard Guo Date: Mon, 27 Jul 2026 17:34:22 +0900 Subject: [PATCH v1] Flush Memoize cache when a buried cache-key parameter changes ExecReScanMemoize() purges the whole cache only when a parameter that is not part of the cache key changes; a change to a cache-key parameter is assumed to be handled by looking up a different cache entry. That assumption holds only when the parameter's entire effect on the subplan's output is captured by the cache key. It fails when a parameter is buried inside a larger cache-key expression and also appears elsewhere in the subplan. For example, with a cache key of "t2.hundred + t0.ten" and a separate filter "t1.twenty = t0.ten", different (t2.hundred, t0.ten) pairs can produce the same cache-key value while the filter selects different rows, so one cache entry wrongly serves combinations that should return different results. Since t0.ten was included in keyparamids, ExecReScanMemoize() failed to flush the cache when it changed, producing wrong results. Fix create_memoize_plan() to drop from keyparamids any parameter that also appears outside the cache-key expressions. A Memoize node always sits atop a scan of a single relation, so the only non-key places such a parameter can appear are that relation's own restriction clauses and targetlist; it is enough to inspect those. A parameter that is itself a complete cache key is exempt, since equality of the key then implies equality of the parameter. --- src/backend/optimizer/plan/createplan.c | 36 ++++++++++++++++++++++ src/include/nodes/execnodes.h | 5 ++-- src/include/nodes/plannodes.h | 5 +++- src/test/regress/expected/memoize.out | 40 +++++++++++++++++++++++++ src/test/regress/sql/memoize.sql | 21 +++++++++++++ 5 files changed, 104 insertions(+), 3 deletions(-) diff --git a/src/backend/optimizer/plan/createplan.c b/src/backend/optimizer/plan/createplan.c index 2c696ea0268..404e8e86c4c 100644 --- a/src/backend/optimizer/plan/createplan.c +++ b/src/backend/optimizer/plan/createplan.c @@ -1718,6 +1718,8 @@ create_memoize_plan(PlannerInfo *root, MemoizePath *best_path, int flags) { Memoize *plan; Bitmapset *keyparamids; + Bitmapset *otherparamids; + RelOptInfo *rel = best_path->subpath->parent; Plan *subplan; Oid *operators; Oid *collations; @@ -1751,6 +1753,40 @@ create_memoize_plan(PlannerInfo *root, MemoizePath *best_path, int flags) keyparamids = pull_paramids((Expr *) param_exprs); + /* + * ExecReScanMemoize() skips purging the cache when only cache-key + * parameters change, which is safe only if a parameter's effect on the + * subplan is fully captured by the cache key. That's not so for a + * parameter buried in a larger cache-key expression that also appears + * elsewhere in the subplan, since two parameter combinations can then + * share a cache-key value yet yield different results. Remove any such + * parameter from keyparamids so its change forces a purge. The Memoize + * node sits atop a single relation's scan, so it suffices to check that + * relation's restriction clauses and targetlist. A parameter that is + * itself a complete cache key is exempt. + */ + otherparamids = NULL; + foreach(lc, rel->baserestrictinfo) + { + RestrictInfo *rinfo = lfirst_node(RestrictInfo, lc); + + otherparamids = bms_add_members(otherparamids, + pull_paramids(rinfo->clause)); + } + otherparamids = bms_add_members(otherparamids, + pull_paramids((Expr *) rel->reltarget->exprs)); + + foreach(lc, param_exprs) + { + Node *param_expr = (Node *) lfirst(lc); + + if (IsA(param_expr, Param)) + otherparamids = bms_del_member(otherparamids, + ((Param *) param_expr)->paramid); + } + + keyparamids = bms_del_members(keyparamids, otherparamids); + plan = make_memoize(subplan, operators, collations, param_exprs, best_path->singlerow, best_path->binary_mode, best_path->est_entries, keyparamids, best_path->est_calls, diff --git a/src/include/nodes/execnodes.h b/src/include/nodes/execnodes.h index e64fd8c7ea3..d22f616d850 100644 --- a/src/include/nodes/execnodes.h +++ b/src/include/nodes/execnodes.h @@ -2332,8 +2332,9 @@ typedef struct MemoizeState * by bit, false when using hash equality ops */ MemoizeInstrumentation stats; /* execution statistics */ SharedMemoizeInfo *shared_info; /* statistics for parallel workers */ - Bitmapset *keyparamids; /* Param->paramids of expressions belonging to - * param_exprs */ + Bitmapset *keyparamids; /* copy of Memoize.keyparamids; a change to + * any param not in this set forces a cache + * purge in ExecReScanMemoize() */ } MemoizeState; /* ---------------- diff --git a/src/include/nodes/plannodes.h b/src/include/nodes/plannodes.h index b880ce0f4be..f142359c09d 100644 --- a/src/include/nodes/plannodes.h +++ b/src/include/nodes/plannodes.h @@ -1124,7 +1124,10 @@ typedef struct Memoize */ uint32 est_entries; - /* paramids from param_exprs */ + /* + * Paramids from param_exprs, minus any that also affect the subplan + * outside the cache keys. See create_memoize_plan(). + */ Bitmapset *keyparamids; /* Estimated number of rescans, for EXPLAIN */ diff --git a/src/test/regress/expected/memoize.out b/src/test/regress/expected/memoize.out index 218972dfab8..27f705257e3 100644 --- a/src/test/regress/expected/memoize.out +++ b/src/test/regress/expected/memoize.out @@ -455,6 +455,46 @@ WHERE unique1 < 3 2 (1 row) +-- Exercise Memoize cache flushing when a parameter that is buried inside a +-- larger cache-key expression also appears in a qual that is not part of the +-- cache key. +-- Ensure we get a Memoize plan with a composite cache key +EXPLAIN (COSTS OFF) +SELECT sum(c) FROM ( + SELECT t0.unique1, + (SELECT count(*) FROM tenk1 t2 + INNER JOIN tenk1 t1 ON t1.unique1 = t2.hundred + t0.ten + WHERE t1.twenty = t0.ten) AS c + FROM tenk1 t0 WHERE t0.unique1 < 2) s; + QUERY PLAN +--------------------------------------------------------------------------- + Aggregate + -> Index Scan using tenk1_unique1 on tenk1 t0 + Index Cond: (unique1 < 2) + SubPlan expr_1 + -> Aggregate + -> Nested Loop + -> Index Only Scan using tenk1_hundred on tenk1 t2 + -> Memoize + Cache Key: (t2.hundred + t0.ten) + Cache Mode: logical + -> Index Scan using tenk1_unique1 on tenk1 t1 + Index Cond: (unique1 = (t2.hundred + t0.ten)) + Filter: (twenty = t0.ten) +(13 rows) + +-- Ensure the above query returns the correct result +SELECT sum(c) FROM ( + SELECT t0.unique1, + (SELECT count(*) FROM tenk1 t2 + INNER JOIN tenk1 t1 ON t1.unique1 = t2.hundred + t0.ten + WHERE t1.twenty = t0.ten) AS c + FROM tenk1 t0 WHERE t0.unique1 < 2) s; + sum +------ + 1000 +(1 row) + RESET enable_seqscan; RESET enable_material; RESET enable_mergejoin; diff --git a/src/test/regress/sql/memoize.sql b/src/test/regress/sql/memoize.sql index e39bbb65391..8764503a9e8 100644 --- a/src/test/regress/sql/memoize.sql +++ b/src/test/regress/sql/memoize.sql @@ -218,6 +218,27 @@ WHERE unique1 < 3 INNER JOIN tenk1 t2 ON t1.unique1 = t2.hundred WHERE t0.ten = t1.twenty AND t0.two <> t2.four OFFSET 0); +-- Exercise Memoize cache flushing when a parameter that is buried inside a +-- larger cache-key expression also appears in a qual that is not part of the +-- cache key. + +-- Ensure we get a Memoize plan with a composite cache key +EXPLAIN (COSTS OFF) +SELECT sum(c) FROM ( + SELECT t0.unique1, + (SELECT count(*) FROM tenk1 t2 + INNER JOIN tenk1 t1 ON t1.unique1 = t2.hundred + t0.ten + WHERE t1.twenty = t0.ten) AS c + FROM tenk1 t0 WHERE t0.unique1 < 2) s; + +-- Ensure the above query returns the correct result +SELECT sum(c) FROM ( + SELECT t0.unique1, + (SELECT count(*) FROM tenk1 t2 + INNER JOIN tenk1 t1 ON t1.unique1 = t2.hundred + t0.ten + WHERE t1.twenty = t0.ten) AS c + FROM tenk1 t0 WHERE t0.unique1 < 2) s; + RESET enable_seqscan; RESET enable_material; RESET enable_mergejoin; -- 2.39.5 (Apple Git-154)