bms_prev_member won't work correctly if bitmapword is 64-bits
bms_prev_member mistakenly has a hardcoded 24 in the function. This
should really be BITS_PER_BITMAPWORD - 8 so that it is properly set to
56 if someone compiles with 64-bit bitmapwords.
The attached fixes this and also adds a test to exercise the function
a bit. [1]https://coverage.postgresql.org/src/backend/nodes/bitmapset.c.gcov.html indicates there's currently no coverage of this function at
all.
[1]: https://coverage.postgresql.org/src/backend/nodes/bitmapset.c.gcov.html
--
David Rowley http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services
Attachments:
0001-Make-bms_prev_member-work-correctly-with-a-64-bit-bi.patchapplication/octet-stream; name=0001-Make-bms_prev_member-work-correctly-with-a-64-bit-bi.patchDownload
From 35aa7596b6d239af432b4caafc6dde4f5aaaa25c Mon Sep 17 00:00:00 2001
From: "dgrowley@gmail.com" <dgrowley@gmail.com>
Date: Sun, 22 Apr 2018 19:17:08 +1200
Subject: [PATCH] Make bms_prev_member work correctly with a 64 bit bitmapword
5c067521 erroneously had coded bms_prev_member assuming that a bitmapword
would always hold 32 bits and started it's search on what it thought was the
highest 8-bits of the word. This was not the case if bitmapwords were 64
bits.
In passing add a test to exercise this function a little. Previously there was
no coverage at all.
---
src/backend/nodes/bitmapset.c | 2 +-
src/test/regress/expected/partition_prune.out | 23 +++++++++++++++++++++++
src/test/regress/sql/partition_prune.sql | 25 +++++++++++++++++++++++++
3 files changed, 49 insertions(+), 1 deletion(-)
diff --git a/src/backend/nodes/bitmapset.c b/src/backend/nodes/bitmapset.c
index 9341bf579e..81182f2518 100644
--- a/src/backend/nodes/bitmapset.c
+++ b/src/backend/nodes/bitmapset.c
@@ -1167,7 +1167,7 @@ bms_prev_member(const Bitmapset *a, int prevbit)
if (w != 0)
{
int result;
- int shift = 24;
+ int shift = BITS_PER_BITMAPWORD - 8;
result = wordnum * BITS_PER_BITMAPWORD;
while ((w >> shift) == 0)
diff --git a/src/test/regress/expected/partition_prune.out b/src/test/regress/expected/partition_prune.out
index 9c65ee001d..3e818edd33 100644
--- a/src/test/regress/expected/partition_prune.out
+++ b/src/test/regress/expected/partition_prune.out
@@ -1747,6 +1747,29 @@ explain (analyze, costs off, summary off, timing off) execute ab_q3 (2, 2);
Filter: ((b >= $1) AND (b <= $2) AND (a < $0))
(10 rows)
+-- Test a backwards Append scan
+create table list_part (a int) partition by list (a);
+create table list_part1 partition of list_part for values in (1);
+create table list_part2 partition of list_part for values in (2);
+create table list_part3 partition of list_part for values in (3);
+create table list_part4 partition of list_part for values in (4);
+insert into list_part select generate_series(1,4);
+begin;
+-- Don't select an actual value out of the table as the order of the Append's
+-- subnodes may not be stable.
+declare cur SCROLL CURSOR for select 1 from list_part where a > (select 1) and a < (select 4);
+-- move beyond the final row
+move 3 from cur;
+-- Ensure we get two rows.
+fetch backward all from cur;
+ ?column?
+----------
+ 1
+ 1
+(2 rows)
+
+commit;
+drop table list_part;
-- Parallel append
-- Suppress the number of loops each parallel node runs for. This is because
-- more than one worker may run the same parallel node if timing conditions
diff --git a/src/test/regress/sql/partition_prune.sql b/src/test/regress/sql/partition_prune.sql
index b38b39c71e..d8d3e3c47d 100644
--- a/src/test/regress/sql/partition_prune.sql
+++ b/src/test/regress/sql/partition_prune.sql
@@ -359,6 +359,31 @@ execute ab_q3 (1, 8);
explain (analyze, costs off, summary off, timing off) execute ab_q3 (2, 2);
+-- Test a backwards Append scan
+create table list_part (a int) partition by list (a);
+create table list_part1 partition of list_part for values in (1);
+create table list_part2 partition of list_part for values in (2);
+create table list_part3 partition of list_part for values in (3);
+create table list_part4 partition of list_part for values in (4);
+
+insert into list_part select generate_series(1,4);
+
+begin;
+
+-- Don't select an actual value out of the table as the order of the Append's
+-- subnodes may not be stable.
+declare cur SCROLL CURSOR for select 1 from list_part where a > (select 1) and a < (select 4);
+
+-- move beyond the final row
+move 3 from cur;
+
+-- Ensure we get two rows.
+fetch backward all from cur;
+
+commit;
+
+drop table list_part;
+
-- Parallel append
-- Suppress the number of loops each parallel node runs for. This is because
--
2.16.2.windows.1
Thank you, pushed
David Rowley wrote:
bms_prev_member mistakenly has a hardcoded 24 in the function. This
should really be BITS_PER_BITMAPWORD - 8 so that it is properly set to
56 if someone compiles with 64-bit bitmapwords.The attached fixes this and also adds a test to exercise the function
a bit. [1] indicates there's currently no coverage of this function at
all.[1] https://coverage.postgresql.org/src/backend/nodes/bitmapset.c.gcov.html
--
Teodor Sigaev E-mail: teodor@sigaev.ru
WWW: http://www.sigaev.ru/
On 24 April 2018 at 03:00, Teodor Sigaev <teodor@sigaev.ru> wrote:
Thank you, pushed
Thanks!
--
David Rowley http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services