From 1a527fa9521265592024d85caaa22a4e44ec565b Mon Sep 17 00:00:00 2001 From: David Rowley Date: Mon, 27 Jul 2026 23:18:52 +1200 Subject: [PATCH v1] Fix missing Memoize Cache keys for base qual Params This fixes a rare possibility that Memoize could produce wrong results if a Memoize cache key is made up of an expression that contains a Param which the memoized results depend upon, but that Param is not part of the memoization key by itself (as a Var). While nodeMemoize.c was coded to flush the cache when a Param changed which isn't part of the Memoize key, that could fail in circumstances where the missing Param is also a component part of the cache key as an OpExpr. Here we fix this by looking through the base quals to check for Params from above or from the outer side of the join and include those as part of the cache key, if they're not already. Reported-by: Jacob Brazeal Discussion: https://postgr.es/m/CA+COZaAHZT7T2G2F0-drdQB+StWJxOzqeDxp=ESp+YUiy-kOaQ@mail.gmail.com --- src/backend/optimizer/path/joinpath.c | 58 +++++++++++++++++++++++++++ src/test/regress/expected/memoize.out | 48 ++++++++++++++++++++-- src/test/regress/sql/memoize.sql | 24 +++++++++-- 3 files changed, 123 insertions(+), 7 deletions(-) diff --git a/src/backend/optimizer/path/joinpath.c b/src/backend/optimizer/path/joinpath.c index 713283a73aa..40280d0ab38 100644 --- a/src/backend/optimizer/path/joinpath.c +++ b/src/backend/optimizer/path/joinpath.c @@ -17,6 +17,7 @@ #include "executor/executor.h" #include "foreign/fdwapi.h" #include "nodes/nodeFuncs.h" +#include "optimizer/clauses.h" #include "optimizer/cost.h" #include "optimizer/optimizer.h" #include "optimizer/pathnode.h" @@ -559,6 +560,63 @@ paraminfo_get_equal_hashops(PlannerInfo *root, ParamPathInfo *param_info, } } + /* Check for parameters in the base quals. */ + foreach(lc, innerrel->baserestrictinfo) + { + RestrictInfo *rinfo = (RestrictInfo *) lfirst(lc); + OpExpr *opexpr = (OpExpr *) rinfo->clause; + Node *expr; + + /* Skip quals without parameters */ + if (bms_is_empty(pull_paramids((Expr *) opexpr))) + continue; + + /* + * Check we've got an OpExpr with two args that is part of the join + * condition. If not, we can't use memoize for this join. + */ + if (!IsA(opexpr, OpExpr) || list_length(opexpr->args) != 2 || + !clause_sides_match_join(rinfo, outerrel->relids, + innerrel->relids)) + { + list_free(*operators); + list_free(*param_exprs); + return false; + } + + if (rinfo->outer_is_left) + expr = (Node *) linitial(opexpr->args); + else + expr = (Node *) lsecond(opexpr->args); + + if (!list_member(*param_exprs, expr)) + { + TypeCacheEntry *typentry; + + typentry = lookup_type_cache(exprType(expr), + TYPECACHE_HASH_PROC | + TYPECACHE_EQ_OPR); + + if (!OidIsValid(typentry->hash_proc) || + !OidIsValid(typentry->eq_opr)) + { + list_free(*operators); + list_free(*param_exprs); + return false; + } + + *operators = lappend_oid(*operators, typentry->eq_opr); + *param_exprs = lappend(*param_exprs, expr); + + /* + * Force the cache into binary mode unless the equality operator + * we found matches the OpExpr's. + */ + if (typentry->eq_opr != opexpr->opno) + *binary_mode = true; + } + } + /* Now add any lateral vars to the cache key too */ lateral_vars = list_concat(ph_lateral_vars, innerrel->lateral_vars); foreach(lc, lateral_vars) diff --git a/src/test/regress/expected/memoize.out b/src/test/regress/expected/memoize.out index 218972dfab8..1901377d7aa 100644 --- a/src/test/regress/expected/memoize.out +++ b/src/test/regress/expected/memoize.out @@ -416,9 +416,9 @@ ON t1.a = t2.a;', false); DROP TABLE prt; RESET enable_partitionwise_join; --- Exercise Memoize code that flushes the cache when a parameter changes which --- is not part of the cache key. --- Ensure we get a Memoize plan +-- Ensure Params which are part of the base quals are also added as a cache +-- key. +-- Ensure we get a Memoize plan with both keys. EXPLAIN (COSTS OFF) SELECT unique1 FROM tenk1 t0 WHERE unique1 < 3 @@ -436,7 +436,7 @@ WHERE unique1 < 3 -> Index Scan using tenk1_hundred on tenk1 t2 Filter: (t0.two <> four) -> Memoize - Cache Key: t2.hundred + Cache Key: t2.hundred, t0.ten Cache Mode: logical -> Index Scan using tenk1_unique1 on tenk1 t1 Index Cond: (unique1 = t2.hundred) @@ -455,6 +455,46 @@ WHERE unique1 < 3 2 (1 row) +-- Try another permutation of the above, but using inequality, and ensure we +-- get binary cache mode. +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: EXISTS(SubPlan exists_1) + SubPlan exists_1 + -> Nested Loop + -> Index Scan using tenk1_hundred on tenk1 t2 + Filter: (t0.two <> four) + -> Memoize + Cache Key: t2.hundred, t0.ten + Cache Mode: binary + -> Index Scan using tenk1_unique1 on tenk1 t1 + Index Cond: (unique1 = t2.hundred) + Filter: (t0.ten <> twenty) +(13 rows) + +-- And check we get the correct results. +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); + unique1 +--------- + 0 + 1 + 2 +(3 rows) + 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..8bb3b61f097 100644 --- a/src/test/regress/sql/memoize.sql +++ b/src/test/regress/sql/memoize.sql @@ -198,10 +198,10 @@ DROP TABLE prt; RESET enable_partitionwise_join; --- Exercise Memoize code that flushes the cache when a parameter changes which --- is not part of the cache key. +-- Ensure Params which are part of the base quals are also added as a cache +-- key. --- Ensure we get a Memoize plan +-- Ensure we get a Memoize plan with both keys. EXPLAIN (COSTS OFF) SELECT unique1 FROM tenk1 t0 WHERE unique1 < 3 @@ -218,6 +218,24 @@ WHERE unique1 < 3 INNER JOIN tenk1 t2 ON t1.unique1 = t2.hundred WHERE t0.ten = t1.twenty AND t0.two <> t2.four OFFSET 0); +-- Try another permutation of the above, but using inequality, and ensure we +-- get binary cache mode. +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); + +-- And check we get the correct results. +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); + RESET enable_seqscan; RESET enable_material; RESET enable_mergejoin; -- 2.53.0