From 5a18c53543f4b2d86e0a71bb9cd03965dcb47b4a Mon Sep 17 00:00:00 2001 From: Richard Guo Date: Mon, 2 Nov 2026 16:59:07 +0900 Subject: [PATCH v1] Make Memoize cache keys only of Params from outer query levels Commit 708eb5158 made each PARAM_EXEC Param buried inside a larger Memoize cache key expression a cache key by itself, and, in master only, commit f8da64e6d extended that to the Params in the memoized relation's base quals, targetlist and join clauses. Both meant to catch Params supplied by an outer query level, but they took every PARAM_EXEC Param they found, including the output Params of initPlans and SubPlans. An initPlan's output Param made a cache key is evaluated on every cache probe, which runs the initPlan even where the query doesn't need it, such as in a CASE arm that is never taken, and can raise an error the query shouldn't raise. A SubPlan's output Params have a value only while the SubPlan evaluates its testexpr, so making a cache key of one fails with "plan should not reference subplan's variable". To fix, make cache keys only of the Params listed in some outer query level's plan_params, as those hold values supplied by outer levels and are always safe to evaluate. Any other Param is left to purge the cache when it changes, as before those commits. Back-patch to all supported branches. --- src/backend/optimizer/path/joinpath.c | 34 ++++++++++++++- src/test/regress/expected/memoize.out | 59 +++++++++++++++++++++++++++ src/test/regress/sql/memoize.sql | 24 +++++++++++ 3 files changed, 115 insertions(+), 2 deletions(-) diff --git a/src/backend/optimizer/path/joinpath.c b/src/backend/optimizer/path/joinpath.c index b690deb6275..a70a3ba716a 100644 --- a/src/backend/optimizer/path/joinpath.c +++ b/src/backend/optimizer/path/joinpath.c @@ -487,6 +487,8 @@ paraminfo_get_equal_hashops(PlannerInfo *root, ParamPathInfo *param_info, { List *lateral_vars; List *params; + Bitmapset *outer_paramids; + PlannerInfo *proot; ListCell *lc; *param_exprs = NIL; @@ -597,7 +599,30 @@ paraminfo_get_equal_hashops(PlannerInfo *root, ParamPathInfo *param_info, * key of the Param by itself so that the cached results survive. This is * only an optimization, so we needn't track down every last such Param, * nor refuse to memoize when one isn't hashable. + * + * Only Params supplied by an outer query level are safe to evaluate as + * cache keys. Others, such as the output Params of initPlans and + * SubPlans, may not have a value yet. Outer query levels list the Params + * they supply in their plan_params. */ + if (root->glob->paramExecTypes == NIL) + return true; /* no PARAM_EXEC Params at all */ + + outer_paramids = NULL; + for (proot = root->parent_root; proot != NULL; proot = proot->parent_root) + { + foreach(lc, proot->plan_params) + { + PlannerParamItem *pitem = lfirst_node(PlannerParamItem, lc); + + outer_paramids = bms_add_member(outer_paramids, pitem->paramId); + } + } + + /* Nothing to do if no outer level supplies any Params */ + if (outer_paramids == NULL) + return true; + params = NIL; (void) pull_exec_params_walker((Node *) *param_exprs, ¶ms); foreach(lc, innerrel->baserestrictinfo) @@ -627,8 +652,13 @@ paraminfo_get_equal_hashops(PlannerInfo *root, ParamPathInfo *param_info, } foreach(lc, params) - (void) memoize_add_cache_key((Node *) lfirst(lc), param_exprs, - operators, binary_mode); + { + Param *param = (Param *) lfirst(lc); + + if (bms_is_member(param->paramid, outer_paramids)) + (void) memoize_add_cache_key((Node *) param, param_exprs, + operators, binary_mode); + } /* We're okay to use memoize */ return true; diff --git a/src/test/regress/expected/memoize.out b/src/test/regress/expected/memoize.out index 819cdf41298..30bf5f5667e 100644 --- a/src/test/regress/expected/memoize.out +++ b/src/test/regress/expected/memoize.out @@ -570,6 +570,65 @@ EXECUTE memoize_extern_param(0); DEALLOCATE memoize_extern_param; RESET plan_cache_mode; +-- Ensure an initplan's output Param isn't made a cache key, as that would run +-- the initplan where it's not needed. +-- Ensure we get a Memoize plan without the initplan Param as a cache key +EXPLAIN (COSTS OFF) +SELECT count(*) FROM tenk1 t1 INNER JOIN tenk1 t2 + ON t1.unique1 = COALESCE(t2.twenty, (SELECT 1 / unique1 FROM tenk1 WHERE unique1 = 0)); + QUERY PLAN +----------------------------------------------------------------------------------------- + Aggregate + InitPlan expr_1 + -> Index Only Scan using tenk1_unique1 on tenk1 + Index Cond: (unique1 = 0) + -> Nested Loop + -> Seq Scan on tenk1 t2 + Disabled: true + -> Memoize + Cache Key: COALESCE(t2.twenty, (InitPlan expr_1).col1) + Cache Mode: logical + -> Index Only Scan using tenk1_unique1 on tenk1 t1 + Index Cond: (unique1 = COALESCE(t2.twenty, (InitPlan expr_1).col1)) +(12 rows) + +-- Ensure the above query doesn't run the initplan +SELECT count(*) FROM tenk1 t1 INNER JOIN tenk1 t2 + ON t1.unique1 = COALESCE(t2.twenty, (SELECT 1 / unique1 FROM tenk1 WHERE unique1 = 0)); + count +------- + 10000 +(1 row) + +-- Ensure a SubPlan's output Param isn't made a cache key, as it has no value +-- outside the SubPlan. +-- Ensure we get a Memoize plan without the SubPlan Param as a cache key +EXPLAIN (COSTS OFF) +SELECT count(*) FROM tenk1 t1 INNER JOIN tenk1 t2 + ON t1.unique1 = CASE WHEN t2.twenty < ALL (SELECT 10000) THEN t2.twenty END; + QUERY PLAN +-------------------------------------------------------------------------------------------------------------------------------------- + Aggregate + -> Nested Loop + -> Seq Scan on tenk1 t2 + Disabled: true + -> Memoize + Cache Key: CASE WHEN (ALL (t2.twenty < (SubPlan all_1).col1)) THEN t2.twenty ELSE NULL::integer END + Cache Mode: logical + -> Index Only Scan using tenk1_unique1 on tenk1 t1 + Index Cond: (unique1 = CASE WHEN (ALL (t2.twenty < (SubPlan all_1).col1)) THEN t2.twenty ELSE NULL::integer END) + SubPlan all_1 + -> Result +(11 rows) + +-- Ensure the above query returns the correct result +SELECT count(*) FROM tenk1 t1 INNER JOIN tenk1 t2 + ON t1.unique1 = CASE WHEN t2.twenty < ALL (SELECT 10000) THEN t2.twenty END; + count +------- + 10000 +(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 bf7ea5c05af..594810c37fd 100644 --- a/src/test/regress/sql/memoize.sql +++ b/src/test/regress/sql/memoize.sql @@ -277,6 +277,30 @@ EXECUTE memoize_extern_param(0); DEALLOCATE memoize_extern_param; RESET plan_cache_mode; +-- Ensure an initplan's output Param isn't made a cache key, as that would run +-- the initplan where it's not needed. + +-- Ensure we get a Memoize plan without the initplan Param as a cache key +EXPLAIN (COSTS OFF) +SELECT count(*) FROM tenk1 t1 INNER JOIN tenk1 t2 + ON t1.unique1 = COALESCE(t2.twenty, (SELECT 1 / unique1 FROM tenk1 WHERE unique1 = 0)); + +-- Ensure the above query doesn't run the initplan +SELECT count(*) FROM tenk1 t1 INNER JOIN tenk1 t2 + ON t1.unique1 = COALESCE(t2.twenty, (SELECT 1 / unique1 FROM tenk1 WHERE unique1 = 0)); + +-- Ensure a SubPlan's output Param isn't made a cache key, as it has no value +-- outside the SubPlan. + +-- Ensure we get a Memoize plan without the SubPlan Param as a cache key +EXPLAIN (COSTS OFF) +SELECT count(*) FROM tenk1 t1 INNER JOIN tenk1 t2 + ON t1.unique1 = CASE WHEN t2.twenty < ALL (SELECT 10000) THEN t2.twenty END; + +-- Ensure the above query returns the correct result +SELECT count(*) FROM tenk1 t1 INNER JOIN tenk1 t2 + ON t1.unique1 = CASE WHEN t2.twenty < ALL (SELECT 10000) THEN t2.twenty END; + RESET enable_seqscan; RESET enable_material; RESET enable_mergejoin; -- 2.37.1 (Apple Git-137.1)