Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

Commit7453080

Browse files
committed
Fix incorrect accessing of pfree'd memory in Memoize
For pass-by-reference types, the code added in0b053e7, which aimed toresolve a memory leak, was overly aggressive in resetting the per-tuplememory context which could result in pfree'd memory being accessedresulting in failing to find previously cached results in the hashtable.What was happening was prepare_probe_slot() was switching to theper-tuple memory context and calling ExecEvalExpr(). ExecEvalExpr() mayhave required a memory allocation. Both MemoizeHash_hash() andMemoizeHash_equal() were aggressively resetting the per-tuple contextand after determining the hash value, the context would have gotten resetbefore MemoizeHash_equal() was called. This could have resulted inMemoizeHash_equal() looking at pfree'd memory.This is less likely to have caused issues on a production build as someother allocation would have had to have reused the pfree'd memory tooverwrite it. Otherwise, the original contents would have been intact.However, this clearly caused issues on MEMORY_CONTEXT_CHECKING builds.Author: Tender Wang, Andrei LepikhovReported-by: Tender Wang (using SQLancer)Reviewed-by: Andrei Lepikhov, Richard Guo, David RowleyDiscussion:https://postgr.es/m/CAHewXNnT6N6UJkya0z-jLFzVxcwGfeRQSfhiwA+NyLg-x8iGew@mail.gmail.comBackpatch-through: 14, where Memoize was added
1 parent84788ee commit7453080

File tree

3 files changed

+63
-6
lines changed

3 files changed

+63
-6
lines changed

‎src/backend/executor/nodeMemoize.c‎

Lines changed: 11 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@
1313
* Memoize nodes are intended to sit above parameterized nodes in the plan
1414
* tree in order to cache results from them. The intention here is that a
1515
* repeat scan with a parameter value that has already been seen by the node
16-
* can fetch tuples from the cache rather than having to re-scan theouter
16+
* can fetch tuples from the cache rather than having to re-scan theinner
1717
* node all over again. The query planner may choose to make use of one of
1818
* these when it thinks rescans for previously seen values are likely enough
1919
* to warrant adding the additional node.
@@ -207,7 +207,6 @@ MemoizeHash_hash(struct memoize_hash *tb, const MemoizeKey *key)
207207
}
208208
}
209209

210-
ResetExprContext(econtext);
211210
MemoryContextSwitchTo(oldcontext);
212211
returnmurmurhash32(hashkey);
213212
}
@@ -265,15 +264,14 @@ MemoizeHash_equal(struct memoize_hash *tb, const MemoizeKey *key1,
265264
}
266265
}
267266

268-
ResetExprContext(econtext);
269267
MemoryContextSwitchTo(oldcontext);
270268
returnmatch;
271269
}
272270
else
273271
{
274272
econtext->ecxt_innertuple=tslot;
275273
econtext->ecxt_outertuple=pslot;
276-
returnExecQualAndReset(mstate->cache_eq_expr,econtext);
274+
returnExecQual(mstate->cache_eq_expr,econtext);
277275
}
278276
}
279277

@@ -694,9 +692,18 @@ static TupleTableSlot *
694692
ExecMemoize(PlanState*pstate)
695693
{
696694
MemoizeState*node=castNode(MemoizeState,pstate);
695+
ExprContext*econtext=node->ss.ps.ps_ExprContext;
697696
PlanState*outerNode;
698697
TupleTableSlot*slot;
699698

699+
CHECK_FOR_INTERRUPTS();
700+
701+
/*
702+
* Reset per-tuple memory context to free any expression evaluation
703+
* storage allocated in the previous tuple cycle.
704+
*/
705+
ResetExprContext(econtext);
706+
700707
switch (node->mstatus)
701708
{
702709
caseMEMO_CACHE_LOOKUP:

‎src/test/regress/expected/memoize.out‎

Lines changed: 30 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -92,10 +92,39 @@ WHERE t1.unique1 < 1000;
9292
1000 | 9.5000000000000000
9393
(1 row)
9494

95+
SET enable_mergejoin TO off;
96+
-- Test for varlena datatype with expr evaluation
97+
CREATE TABLE expr_key (x numeric, t text);
98+
INSERT INTO expr_key (x, t)
99+
SELECT d1::numeric, d1::text FROM (
100+
SELECT round((d / pi())::numeric, 7) AS d1 FROM generate_series(1, 20) AS d
101+
) t;
102+
-- duplicate rows so we get some cache hits
103+
INSERT INTO expr_key SELECT * FROM expr_key;
104+
CREATE INDEX expr_key_idx_x_t ON expr_key (x, t);
105+
VACUUM ANALYZE expr_key;
106+
-- Ensure we get we get a cache miss and hit for each of the 20 distinct values
107+
SELECT explain_memoize('
108+
SELECT * FROM expr_key t1 INNER JOIN expr_key t2
109+
ON t1.x = t2.t::numeric AND t1.t::numeric = t2.x;', false);
110+
explain_memoize
111+
-------------------------------------------------------------------------------------------
112+
Nested Loop (actual rows=80 loops=N)
113+
-> Seq Scan on expr_key t1 (actual rows=40 loops=N)
114+
-> Memoize (actual rows=2 loops=N)
115+
Cache Key: t1.x, (t1.t)::numeric
116+
Cache Mode: logical
117+
Hits: 20 Misses: 20 Evictions: Zero Overflows: 0 Memory Usage: NkB
118+
-> Index Only Scan using expr_key_idx_x_t on expr_key t2 (actual rows=2 loops=N)
119+
Index Cond: (x = (t1.t)::numeric)
120+
Filter: (t1.x = (t)::numeric)
121+
Heap Fetches: N
122+
(10 rows)
123+
124+
DROP TABLE expr_key;
95125
-- Reduce work_mem and hash_mem_multiplier so that we see some cache evictions
96126
SET work_mem TO '64kB';
97127
SET hash_mem_multiplier TO 1.0;
98-
SET enable_mergejoin TO off;
99128
-- Ensure we get some evictions. We're unable to validate the hits and misses
100129
-- here as the number of entries that fit in the cache at once will vary
101130
-- between different machines.

‎src/test/regress/sql/memoize.sql‎

Lines changed: 22 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -57,10 +57,31 @@ LATERAL (SELECT t2.unique1 FROM tenk1 t2
5757
WHEREt1.twenty=t2.unique1 OFFSET0) t2
5858
WHEREt1.unique1<1000;
5959

60+
SET enable_mergejoin TO off;
61+
62+
-- Test for varlena datatype with expr evaluation
63+
CREATETABLEexpr_key (xnumeric, ttext);
64+
INSERT INTO expr_key (x, t)
65+
SELECT d1::numeric, d1::textFROM (
66+
SELECT round((d/ pi())::numeric,7)AS d1FROM generate_series(1,20)AS d
67+
) t;
68+
69+
-- duplicate rows so we get some cache hits
70+
INSERT INTO expr_keySELECT*FROM expr_key;
71+
72+
CREATEINDEXexpr_key_idx_x_tON expr_key (x, t);
73+
VACUUM ANALYZE expr_key;
74+
75+
-- Ensure we get we get a cache miss and hit for each of the 20 distinct values
76+
SELECT explain_memoize('
77+
SELECT * FROM expr_key t1 INNER JOIN expr_key t2
78+
ON t1.x = t2.t::numeric AND t1.t::numeric = t2.x;', false);
79+
80+
DROPTABLE expr_key;
81+
6082
-- Reduce work_mem and hash_mem_multiplier so that we see some cache evictions
6183
SET work_mem TO'64kB';
6284
SET hash_mem_multiplier TO1.0;
63-
SET enable_mergejoin TO off;
6485
-- Ensure we get some evictions. We're unable to validate the hits and misses
6586
-- here as the number of entries that fit in the cache at once will vary
6687
-- between different machines.

0 commit comments

Comments
 (0)
[8]ページ先頭
©2009-2025 Movatter.jp