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

Commite629846

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 parent21e3a8b commite629846

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

@@ -699,9 +697,18 @@ static TupleTableSlot *
699697
ExecMemoize(PlanState*pstate)
700698
{
701699
MemoizeState*node=castNode(MemoizeState,pstate);
700+
ExprContext*econtext=node->ss.ps.ps_ExprContext;
702701
PlanState*outerNode;
703702
TupleTableSlot*slot;
704703

704+
CHECK_FOR_INTERRUPTS();
705+
706+
/*
707+
* Reset per-tuple memory context to free any expression evaluation
708+
* storage allocated in the previous tuple cycle.
709+
*/
710+
ResetExprContext(econtext);
711+
705712
switch (node->mstatus)
706713
{
707714
caseMEMO_CACHE_LOOKUP:

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

Lines changed: 30 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -129,10 +129,39 @@ WHERE t1.unique1 < 10;
129129
20 | 0.50000000000000000000
130130
(1 row)
131131

132+
SET enable_mergejoin TO off;
133+
-- Test for varlena datatype with expr evaluation
134+
CREATE TABLE expr_key (x numeric, t text);
135+
INSERT INTO expr_key (x, t)
136+
SELECT d1::numeric, d1::text FROM (
137+
SELECT round((d / pi())::numeric, 7) AS d1 FROM generate_series(1, 20) AS d
138+
) t;
139+
-- duplicate rows so we get some cache hits
140+
INSERT INTO expr_key SELECT * FROM expr_key;
141+
CREATE INDEX expr_key_idx_x_t ON expr_key (x, t);
142+
VACUUM ANALYZE expr_key;
143+
-- Ensure we get we get a cache miss and hit for each of the 20 distinct values
144+
SELECT explain_memoize('
145+
SELECT * FROM expr_key t1 INNER JOIN expr_key t2
146+
ON t1.x = t2.t::numeric AND t1.t::numeric = t2.x;', false);
147+
explain_memoize
148+
-------------------------------------------------------------------------------------------
149+
Nested Loop (actual rows=80 loops=N)
150+
-> Seq Scan on expr_key t1 (actual rows=40 loops=N)
151+
-> Memoize (actual rows=2 loops=N)
152+
Cache Key: t1.x, (t1.t)::numeric
153+
Cache Mode: logical
154+
Hits: 20 Misses: 20 Evictions: Zero Overflows: 0 Memory Usage: NkB
155+
-> Index Only Scan using expr_key_idx_x_t on expr_key t2 (actual rows=2 loops=N)
156+
Index Cond: (x = (t1.t)::numeric)
157+
Filter: (t1.x = (t)::numeric)
158+
Heap Fetches: N
159+
(10 rows)
160+
161+
DROP TABLE expr_key;
132162
-- Reduce work_mem and hash_mem_multiplier so that we see some cache evictions
133163
SET work_mem TO '64kB';
134164
SET hash_mem_multiplier TO 1.0;
135-
SET enable_mergejoin TO off;
136165
-- Ensure we get some evictions. We're unable to validate the hits and misses
137166
-- here as the number of entries that fit in the cache at once will vary
138167
-- between different machines.

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

Lines changed: 22 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -74,10 +74,31 @@ LATERAL (
7474
ONt1.two=t2.two
7575
WHEREt1.unique1<10;
7676

77+
SET enable_mergejoin TO off;
78+
79+
-- Test for varlena datatype with expr evaluation
80+
CREATETABLEexpr_key (xnumeric, ttext);
81+
INSERT INTO expr_key (x, t)
82+
SELECT d1::numeric, d1::textFROM (
83+
SELECT round((d/ pi())::numeric,7)AS d1FROM generate_series(1,20)AS d
84+
) t;
85+
86+
-- duplicate rows so we get some cache hits
87+
INSERT INTO expr_keySELECT*FROM expr_key;
88+
89+
CREATEINDEXexpr_key_idx_x_tON expr_key (x, t);
90+
VACUUM ANALYZE expr_key;
91+
92+
-- Ensure we get we get a cache miss and hit for each of the 20 distinct values
93+
SELECT explain_memoize('
94+
SELECT * FROM expr_key t1 INNER JOIN expr_key t2
95+
ON t1.x = t2.t::numeric AND t1.t::numeric = t2.x;', false);
96+
97+
DROPTABLE expr_key;
98+
7799
-- Reduce work_mem and hash_mem_multiplier so that we see some cache evictions
78100
SET work_mem TO'64kB';
79101
SET hash_mem_multiplier TO1.0;
80-
SET enable_mergejoin TO off;
81102
-- Ensure we get some evictions. We're unable to validate the hits and misses
82103
-- here as the number of entries that fit in the cache at once will vary
83104
-- between different machines.

0 commit comments

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