Location via proxy:   [ UP ]  
[Report a bug]   [Manage cookies]                
Skip to content

Commit 2322456

Browse files
committed
Fix memory corruption/crash in ANALYZE.
This fixes an embarrassing oversight I (Andres) made in 737a292, namely missing two place where liverows/deadrows were used when converting those variables to pointers, leading to incrementing the pointer, rather than the value. It's not that actually that easy to trigger a crash: One needs tuples deleted by the current transaction, followed by a tuple deleted in another session, all in one page. Which is presumably why this hasn't been noticed before. Reported-By: Steve Singer Author: Steve Singer Discussion: https://postgr.es/m/c7988239-d42c-ddc4-41db-171b23b35e4f@ssinger.info
1 parent 8b21b41 commit 2322456

File tree

3 files changed

+27
-2
lines changed

3 files changed

+27
-2
lines changed

src/backend/access/heap/heapam_handler.c

+2-2
Original file line numberDiff line numberDiff line change
@@ -1113,11 +1113,11 @@ heapam_scan_analyze_next_tuple(TableScanDesc scan, TransactionId OldestXmin,
11131113
* concurrent transaction never commits.
11141114
*/
11151115
if (TransactionIdIsCurrentTransactionId(HeapTupleHeaderGetUpdateXid(targtuple->t_data)))
1116-
deadrows += 1;
1116+
*deadrows += 1;
11171117
else
11181118
{
11191119
sample_it = true;
1120-
liverows += 1;
1120+
*liverows += 1;
11211121
}
11221122
break;
11231123

src/test/regress/expected/vacuum.out

+12
Original file line numberDiff line numberDiff line change
@@ -71,6 +71,18 @@ ANALYZE vaccluster;
7171
ERROR: ANALYZE cannot be executed from VACUUM or ANALYZE
7272
CONTEXT: SQL function "do_analyze" statement 1
7373
SQL function "wrap_do_analyze" statement 1
74+
-- Test ANALYZE in transaction, where the transaction surrounding
75+
-- analyze performed modifications. This tests for the bug at
76+
-- https://postgr.es/m/c7988239-d42c-ddc4-41db-171b23b35e4f%40ssinger.info
77+
-- (which hopefully is unlikely to be reintroduced), but also seems
78+
-- independently worthwhile to cover.
79+
INSERT INTO vactst SELECT generate_series(1, 300);
80+
DELETE FROM vactst WHERE i % 7 = 0; -- delete a few rows outside
81+
BEGIN;
82+
INSERT INTO vactst SELECT generate_series(301, 400);
83+
DELETE FROM vactst WHERE i % 5 <> 0; -- delete a few rows inside
84+
ANALYZE vactst;
85+
COMMIT;
7486
VACUUM FULL pg_am;
7587
VACUUM FULL pg_class;
7688
VACUUM FULL pg_database;

src/test/regress/sql/vacuum.sql

+13
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,19 @@ CREATE INDEX ON vaccluster(wrap_do_analyze(i));
5454
INSERT INTO vaccluster VALUES (1), (2);
5555
ANALYZE vaccluster;
5656

57+
-- Test ANALYZE in transaction, where the transaction surrounding
58+
-- analyze performed modifications. This tests for the bug at
59+
-- https://postgr.es/m/c7988239-d42c-ddc4-41db-171b23b35e4f%40ssinger.info
60+
-- (which hopefully is unlikely to be reintroduced), but also seems
61+
-- independently worthwhile to cover.
62+
INSERT INTO vactst SELECT generate_series(1, 300);
63+
DELETE FROM vactst WHERE i % 7 = 0; -- delete a few rows outside
64+
BEGIN;
65+
INSERT INTO vactst SELECT generate_series(301, 400);
66+
DELETE FROM vactst WHERE i % 5 <> 0; -- delete a few rows inside
67+
ANALYZE vactst;
68+
COMMIT;
69+
5770
VACUUM FULL pg_am;
5871
VACUUM FULL pg_class;
5972
VACUUM FULL pg_database;

0 commit comments

Comments
 (0)