From 55723ecdd1e1ff0c0c3438827c05cb1a29b671e0 Mon Sep 17 00:00:00 2001 From: "tender.wang" Date: Fri, 1 Mar 2024 14:49:11 +0800 Subject: [PATCH v6] 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 +++++++++++++---- src/test/regress/expected/memoize.out | 25 +++++++++++++++++++++++++ src/test/regress/sql/memoize.sql | 14 ++++++++++++++ 3 files changed, 52 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) { diff --git a/src/test/regress/expected/memoize.out b/src/test/regress/expected/memoize.out index cf6886a288..7667ae393c 100644 --- a/src/test/regress/expected/memoize.out +++ b/src/test/regress/expected/memoize.out @@ -196,6 +196,31 @@ SELECT * FROM flt f1 INNER JOIN flt f2 ON f1.f >= f2.f;', false); (10 rows) DROP TABLE flt; +CREATE TABLE expr_key (x numeric, t text); +INSERT INTO expr_key (x,t) SELECT d1::numeric, d1::text FROM ( + SELECT round((d/pi())::numeric,7) AS d1 FROM generate_series(1,20) AS d); +INSERT INTO expr_key SELECT * FROM expr_key; +CREATE INDEX expr_key_idx_x_t ON expr_key (x,t); +VACUUM ANALYZE expr_key; +SELECT explain_memoize(' +SELECT * FROM expr_key t1 INNER JOIN expr_key t2 +ON t1.x=t2.t::numeric AND t1.t::numeric=t2.x;', true); + explain_memoize +------------------------------------------------------------------------------------------- + Nested Loop (actual rows=80 loops=N) + -> Index Only Scan using expr_key_idx_x_t on expr_key t1 (actual rows=40 loops=N) + Heap Fetches: N + -> Memoize (actual rows=2 loops=N) + Cache Key: t1.x, (t1.t)::numeric + Cache Mode: logical + Hits: N Misses: N Evictions: Zero Overflows: 0 Memory Usage: NkB + -> Index Only Scan using expr_key_idx_x_t on expr_key t2 (actual rows=2 loops=N) + Index Cond: (x = (t1.t)::numeric) + Filter: (t1.x = (t)::numeric) + Heap Fetches: N +(11 rows) + +DROP TABLE expr_key; -- Exercise Memoize in binary mode with a large fixed width type and a -- varlena type. CREATE TABLE strtest (n name, t text); diff --git a/src/test/regress/sql/memoize.sql b/src/test/regress/sql/memoize.sql index 1f4ab0ba3b..00003e3cff 100644 --- a/src/test/regress/sql/memoize.sql +++ b/src/test/regress/sql/memoize.sql @@ -103,6 +103,20 @@ SELECT * FROM flt f1 INNER JOIN flt f2 ON f1.f >= f2.f;', false); DROP TABLE flt; +CREATE TABLE expr_key (x numeric, t text); +INSERT INTO expr_key (x,t) SELECT d1::numeric, d1::text FROM ( + SELECT round((d/pi())::numeric,7) AS d1 FROM generate_series(1,20) AS d); + +INSERT INTO expr_key SELECT * FROM expr_key; +CREATE INDEX expr_key_idx_x_t ON expr_key (x,t); +VACUUM ANALYZE expr_key; + +SELECT explain_memoize(' +SELECT * FROM expr_key t1 INNER JOIN expr_key t2 +ON t1.x=t2.t::numeric AND t1.t::numeric=t2.x;', true); + +DROP TABLE expr_key; + -- Exercise Memoize in binary mode with a large fixed width type and a -- varlena type. CREATE TABLE strtest (n name, t text); -- 2.25.1