Skip to content

Commit 24da5b2

Browse files
anarazelanarazel
committed
Add test for HeapBitmapScan's broken skip_fetch optimization
In the previous commit HeapBitmapScan's skip_fetch optimization was removed, due to being broken in not easily fixable ways. Add a test that verifies we don't re-introduce this bug if somebody tries to re-add the feature. Only add the test to master for now, it's possible it's not entirely stable. That seems sufficient, as we're not going to re-introduce the feature on the backbranches. I did verify that the test passes on all branches. If the test turns out to be unproblematic, we can backpatch it later, should we feel a need to do so. Discussion: https://postgr.es/m/CAEze2Wg3gXXZTr6_rwC+s4-o2ZVFB5F985uUSgJTsECx6AmGcQ@mail.gmail.com
1 parent 459e7bf commit 24da5b2

File tree

3 files changed

+157
-0
lines changed

3 files changed

+157
-0
lines changed

‎src/test/isolation/expected/index-only-bitmapscan.out

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,48 @@
1+
Parsed test spec with 2 sessions
2+
3+
starting permutation: s2_vacuum s2_mod s1_explain s1_begin s1_prepare s1_fetch_1 s2_vacuum s1_fetch_all s1_commit
4+
step s2_vacuum:
5+
VACUUM (TRUNCATE false) ios_bitmap;
6+
7+
step s2_mod:
8+
DELETE FROM ios_bitmap WHERE a > 1;
9+
10+
step s1_explain:
11+
EXPlAIN (COSTS OFF) DECLARE foo NO SCROLL CURSOR FOR SELECT row_number() OVER () FROM ios_bitmap WHERE a > 0 or b > 0;
12+
13+
QUERY PLAN
14+
---------------------------------------------------
15+
WindowAgg
16+
Window: w1 AS (ROWS UNBOUNDED PRECEDING)
17+
-> Bitmap Heap Scan on ios_bitmap
18+
Recheck Cond: ((a > 0) OR (b > 0))
19+
-> BitmapOr
20+
-> Bitmap Index Scan on ios_bitmap_a
21+
Index Cond: (a > 0)
22+
-> Bitmap Index Scan on ios_bitmap_b
23+
Index Cond: (b > 0)
24+
(9 rows)
25+
26+
step s1_begin: BEGIN;
27+
step s1_prepare:
28+
DECLARE foo NO SCROLL CURSOR FOR SELECT row_number() OVER () FROM ios_bitmap WHERE a > 0 or b > 0;
29+
30+
step s1_fetch_1:
31+
FETCH FROM foo;
32+
33+
row_number
34+
----------
35+
1
36+
(1 row)
37+
38+
step s2_vacuum:
39+
VACUUM (TRUNCATE false) ios_bitmap;
40+
41+
step s1_fetch_all:
42+
FETCH ALL FROM foo;
43+
44+
row_number
45+
----------
46+
(0 rows)
47+
48+
step s1_commit: COMMIT;

‎src/test/isolation/isolation_schedule

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ test: partial-index
1717
test: two-ids
1818
test: multiple-row-versions
1919
test: index-only-scan
20+
test: index-only-bitmapscan
2021
test: predicate-lock-hot-tuple
2122
test: update-conflict-out
2223
test: deadlock-simple

‎src/test/isolation/specs/index-only-bitmapscan.spec

Lines changed: 108 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,108 @@
1+
# In the past we supported index-only bitmap heapscans. However the
2+
# implementation was unsound, see
3+
# https://postgr.es/m/873c33c5-ef9e-41f6-80b2-2f5e11869f1c%40garret.ru
4+
#
5+
# This test reliably triggered the problem before we removed the
6+
# optimization. We keep the test around to make it less likely for a similar
7+
# problem to be re-introduced.
8+
9+
setup
10+
{
11+
-- by using a low fillfactor and a wide tuple we can get multiple blocks
12+
-- with just few rows
13+
CREATE TABLE ios_bitmap (a int NOT NULL, b int not null, pad char(1024) default '')
14+
WITH (AUTOVACUUM_ENABLED = false, FILLFACTOR = 10);
15+
16+
INSERT INTO ios_bitmap SELECT g.i, g.i FROM generate_series(1, 10) g(i);
17+
18+
CREATE INDEX ios_bitmap_a ON ios_bitmap(a);
19+
CREATE INDEX ios_bitmap_b ON ios_bitmap(b);
20+
}
21+
22+
teardown
23+
{
24+
DROP TABLE ios_bitmap;
25+
}
26+
27+
28+
session s1
29+
30+
setup
31+
{
32+
SET enable_seqscan = false;
33+
}
34+
35+
step s1_begin { BEGIN; }
36+
step s1_commit { COMMIT; }
37+
38+
39+
# The test query uses an or between two indexes to ensure make it more likely
40+
# to use a bitmap index scan
41+
#
42+
# The row_number() hack is a way to have something returned (isolationtester
43+
# doesn't display empty rows) while still allowing for the index-only scan
44+
# optimization in bitmap heap scans, which requires an empty targetlist.
45+
step s1_prepare
46+
{
47+
DECLARE foo NO SCROLL CURSOR FOR SELECT row_number() OVER () FROM ios_bitmap WHERE a > 0 or b > 0;
48+
}
49+
50+
step s1_explain
51+
{
52+
EXPlAIN (COSTS OFF) DECLARE foo NO SCROLL CURSOR FOR SELECT row_number() OVER () FROM ios_bitmap WHERE a > 0 or b > 0;
53+
}
54+
55+
step s1_fetch_1
56+
{
57+
FETCH FROM foo;
58+
}
59+
60+
step s1_fetch_all
61+
{
62+
FETCH ALL FROM foo;
63+
}
64+
65+
66+
session s2
67+
68+
# Don't delete row 1 so we have a row for the cursor to "rest" on.
69+
step s2_mod
70+
{
71+
DELETE FROM ios_bitmap WHERE a > 1;
72+
}
73+
74+
# Disable truncation, as otherwise we'll just wait for a timeout while trying
75+
# to acquire the lock
76+
step s2_vacuum
77+
{
78+
VACUUM (TRUNCATE false) ios_bitmap;
79+
}
80+
81+
permutation
82+
# Vacuum first, to ensure VM exists, otherwise the bitmapscan will consider
83+
# VM to be size 0, due to caching. Can't do that in setup because
84+
s2_vacuum
85+
86+
# Delete nearly all rows, to make issue visible
87+
s2_mod
88+
89+
# Verify that the appropriate plan is chosen
90+
s1_explain
91+
92+
# Create a cursor
93+
s1_begin
94+
s1_prepare
95+
96+
# Fetch one row from the cursor, that ensures the index scan portion is done
97+
# before the vacuum in the next step
98+
s1_fetch_1
99+
100+
# With the bug this vacuum would have marked pages as all-visible that the
101+
# scan in the next step then would have considered all-visible, despite all
102+
# rows from those pages having been removed.
103+
s2_vacuum
104+
105+
# If this returns any rows, the bug is present
106+
s1_fetch_all
107+
108+
s1_commit

0 commit comments

Comments
 (0)