From 8497ae88c203d751013357d32a7940fd353b46c0 Mon Sep 17 00:00:00 2001 From: "tender.wang" Date: Thu, 29 Feb 2024 12:31:34 +0800 Subject: [PATCH v5] Fix wrong used ResetExprContext() in ExecMemoize(). In commit 0b053e78, ResetExprContext() was called in Hash and Equal func. The memory allocation for probeslot is also from per_tuple_memory. It will be safe for probeslot if the data in probeslot is pass-by-value or plain var referencd outertuple slot. But it will be unsafe if the data in probeslot is pass-by-reference and pointer to per_tuple_memory. per_tuple_memory is a short-term context for expression results. As the name suggests, it will typically be reset once per tuple, before we begin to evaluate expressions for that tuple. So we call RestExprContext() every time when we enter ExecMemoize(), ohter function doesn't call it anymore. This way is as same as ExecProjectSet and ExecResult does. --- src/backend/executor/nodeMemoize.c | 17 +++++++++++++---- 1 file changed, 13 insertions(+), 4 deletions(-) diff --git a/src/backend/executor/nodeMemoize.c b/src/backend/executor/nodeMemoize.c index 18870f10e1..4b544afc6c 100644 --- a/src/backend/executor/nodeMemoize.c +++ b/src/backend/executor/nodeMemoize.c @@ -13,7 +13,7 @@ * Memoize nodes are intended to sit above parameterized nodes in the plan * tree in order to cache results from them. The intention here is that a * repeat scan with a parameter value that has already been seen by the node - * can fetch tuples from the cache rather than having to re-scan the outer + * can fetch tuples from the cache rather than having to re-scan the inner * node all over again. The query planner may choose to make use of one of * these when it thinks rescans for previously seen values are likely enough * to warrant adding the additional node. @@ -207,7 +207,6 @@ MemoizeHash_hash(struct memoize_hash *tb, const MemoizeKey *key) } } - ResetExprContext(econtext); MemoryContextSwitchTo(oldcontext); return murmurhash32(hashkey); } @@ -265,7 +264,6 @@ MemoizeHash_equal(struct memoize_hash *tb, const MemoizeKey *key1, } } - ResetExprContext(econtext); MemoryContextSwitchTo(oldcontext); return match; } @@ -273,7 +271,7 @@ MemoizeHash_equal(struct memoize_hash *tb, const MemoizeKey *key1, { econtext->ecxt_innertuple = tslot; econtext->ecxt_outertuple = pslot; - return ExecQualAndReset(mstate->cache_eq_expr, econtext); + return ExecQual(mstate->cache_eq_expr, econtext); } } @@ -701,6 +699,17 @@ ExecMemoize(PlanState *pstate) MemoizeState *node = castNode(MemoizeState, pstate); PlanState *outerNode; TupleTableSlot *slot; + ExprContext *econtext; + + CHECK_FOR_INTERRUPTS(); + + econtext = node->ss.ps.ps_ExprContext; + + /* + * Reset per-tuple memory context to free any expression evaluation + * storage allocated in the previous tuple cycle. + */ + ResetExprContext(econtext); switch (node->mstatus) { -- 2.25.1