10000 Fix bulk table extension when copying into multiple partitions · postgrespro/postgres@22655aa · GitHub
[go: up one dir, main page]
Skip to content

Commit 22655aa

Browse files
anarazelanarazel
committed
Fix bulk table extension when copying into multiple partitions
When COPYing into a partitioned table that does now permit the use of table_multi_insert(), we could error out with ERROR: could not read block NN in file "base/...": read only 0 of 8192 bytes because BulkInsertState->next_free was not reset between partitions. This problem occurred only when not able to use table_multi_insert(), as a dedicated BulkInsertState for each partition is used in that case. The bug was introduced in 00d1e02, but it was hard to hit at that point, as commonly bulk relation extension is not used when not using table_multi_insert(). It became more likely after 82a4eda, which expanded the use of bulk extension. To fix the bug, reset the bulk relation extension state in BulkInsertState in ReleaseBulkInsertStatePin(). That was added (in b1ecb9b) to tackle a very similar issue. Obviously the name is not quite correct, but there might be external callers, and bulk insert state needs to be reset in precisely in the situations that ReleaseBulkInsertStatePin() already needed to be called. Medium term the better fix likely is to disallow reusing BulkInsertState across relations. Add a test that, without the fix, reproduces #18130 in most configurations. The test also catches the problem fixed in b1ecb9b when run with small shared_buffers. Reported-by: Ivan Kolombet <enderstd@gmail.com> Analyzed-by: Tom Lane <tgl@sss.pgh.pa.us> Analyzed-by: Andres Freund <andres@anarazel.de> Bug: #18130 Discussion: https://postgr.es/m/18130-7a86a7356a75209d%40postgresql.org Discussion: https://postgr.es/m/257696.1695670946%40sss.pgh.pa.us Backpatch: 16-
1 parent 8d140c5 commit 22655aa

File tree

3 files changed

+85
-0
lines changed

3 files changed

+85
-0
lines changed

‎src/backend/access/heap/heapam.c

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1792,6 +1792,17 @@ ReleaseBulkInsertStatePin(BulkInsertState bistate)
17921792
if (bistate->current_buf != InvalidBuffer)
17931793
ReleaseBuffer(bistate->current_buf);
17941794
bistate->current_buf = InvalidBuffer;
1795+
1796+
/*
1797+
* Despite the name, we also reset bulk relation extension
1798+
* state. Otherwise we can end up erroring out due to looking for free
1799+
* space in ->next_free of one partition, even though ->next_free was set
1800+
* when extending another partition. It's obviously also could be bad for
1801+
* efficiency to look at existing blocks at offsets from another
1802+
* partition, even if we don't error out.
1803+
*/
1804+
bistate->next_free = InvalidBlockNumber;
1805+
bistate->last_free = InvalidBlockNumber;
17951806
}
17961807

17971808

‎src/test/regr 8000 ess/expected/copy.out

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -257,3 +257,40 @@ ERROR: value too long for type character varying(5)
257257
\.
258258
invalid command \.
259259
drop table oversized_column_default;
260+
--
261+
-- Create partitioned table that does not allow bulk insertions, to test bugs
262+
-- related to the reuse of BulkInsertState across partitions (only done when
263+
-- not using bulk insert). Switching between partitions often makes it more
264+
-- likely to encounter these bugs, so we just switch on roughly every insert
265+
-- by having an even/odd number partition and inserting evenly distributed
266+
-- data.
267+
--
268+
CREATE TABLE parted_si (
269+
id int not null,
270+
data text not null,
271+
-- prevent use of bulk insert by having a volatile function
272+
rand float8 not null default random()
273+
)
274+
PARTITION BY LIST((id % 2));
275+
CREATE TABLE parted_si_p_even PARTITION OF parted_si FOR VALUES IN (0);
276+
CREATE TABLE parted_si_p_odd PARTITION OF parted_si FOR VALUES IN (1);
277+
-- Test that bulk relation extension handles reusing a single BulkInsertState
278+
-- across partitions. Without the fix applied, this reliably reproduces
279+
-- #18130 unless shared_buffers is extremely small (preventing any use use of
280+
-- bulk relation extension). See
281+
-- https://postgr.es/m/18130-7a86a7356a75209d%40postgresql.org
282+
-- https://postgr.es/m/257696.1695670946%40sss.pgh.pa.us
283+
\set filename :abs_srcdir '/data/desc.data'
284+
COPY parted_si(id, data) FROM :'filename';
285+
-- An earlier bug (see commit b1ecb9b3fcf) could end up using a buffer from
286+
-- the wrong partition. This test is *not* guaranteed to trigger that bug, but
287+
-- does so when shared_buffers is small enough. To test if we encountered the
288+
-- bug, check that the partition condition isn't violated.
289+
SELECT tableoid::regclass, id % 2 = 0 is_even, count(*) from parted_si GROUP BY 1, 2 ORDER BY 1;
290+
tableoid | is_even | count
291+
------------------+---------+-------
292+
parted_si_p_even | t | 5000
293+
parted_si_p_odd | f | 5000
294+
(2 rows)
295+
296+
DROP TABLE parted_si;

‎src/test/regress/sql/copy.sql

Copy file name to clipboard
Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -283,3 +283,40 @@ copy oversized_column_default (col2) from stdin;
283283
copy oversized_column_default from stdin (default '');
284284
\.
285285
drop table oversized_column_default;
286+
287+
288+
--
289+
-- Create partitioned table that does not allow bulk insertions, to test bugs
290+
-- related to the reuse of BulkInsertState across partitions (only done when
291+
-- not using bulk insert). Switching between partitions often makes it more
292+
-- likely to encounter these bugs, so we just switch on roughly every insert
293+
-- by having an even/odd number partition and inserting evenly distributed
294+
-- data.
295+
--
296+
CREATE TABLE parted_si (
297+
id int not null,
298+
data text not null,
299+
-- prevent use of bulk insert by having a volatile function
300+
rand float8 not null default random()
301+
)
302+
PARTITION BY LIST((id % 2));
303+
304+
CREATE TABLE parted_si_p_even PARTITION OF parted_si FOR VALUES IN (0);
305+
CREATE TABLE parted_si_p_odd PARTITION OF parted_si FOR VALUES IN (1);
306+
307+
-- Test that bulk relation extension handles reusing a single BulkInsertState
308+
-- across partitions. Without the fix applied, this reliably reproduces
309+
-- #18130 unless shared_buffers is extremely small (preventing any use use of
310+
-- bulk relation extension). See
311+
-- https://postgr.es/m/18130-7a86a7356a75209d%40postgresql.org
312+
-- https://postgr.es/m/257696.1695670946%40sss.pgh.pa.us
313+
\set filename :abs_srcdir '/data/desc.data'
314+
COPY parted_si(id, data) FROM :'filename';
315+
316+
-- An earlier bug (see commit b1ecb9b3fcf) could end up using a buffer from
317+
-- the wrong partition. This test is *not* guaranteed to trigger that bug, but
318+
-- does so when shared_buffers is small enough. To test if we encountered the
319+
-- bug, check that the partition condition isn't violated.
320+
SELECT tableoid::regclass, id % 2 = 0 is_even, count(*) from parted_si GROUP BY 1, 2 ORDER BY 1;
321+
322+
DROP TABLE parted_si;

0 commit comments

Comments
 (0)
0