parallel bitmapscan isn't exercised in regression tests
Hi,
The parallel code-path isn't actually exercised in the tests added in
[1]: https://git.postgresql.org/gitweb/?p=postgresql.git;a=commit;h=f35742ccb7aa53ee3ed8416bbb378b0c3eeb6bb9
fixed.
Greetings,
Andres Freund
[1]: https://git.postgresql.org/gitweb/?p=postgresql.git;a=commit;h=f35742ccb7aa53ee3ed8416bbb378b0c3eeb6bb9
[2]: https://coverage.postgresql.org/src/backend/executor/nodeBitmapHeapscan.c.gcov.html
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On Sat, Apr 1, 2017 at 12:16 AM, Andres Freund <andres@anarazel.de> wrote:
Hi,
The parallel code-path isn't actually exercised in the tests added in
[1], as evidenced by [2] (they just explain). That imo needs to be
fixed.
Thanks for reporting. Attached patch fixes that.
--
Regards,
Dilip Kumar
EnterpriseDB: http://www.enterprisedb.com
Attachments:
parallel_bitmap_test.patchapplication/octet-stream; name=parallel_bitmap_test.patchDownload
diff --git a/src/test/regress/expected/select_parallel.out b/src/test/regress/expected/select_parallel.out
index 038a62e..5d06a9b 100644
--- a/src/test/regress/expected/select_parallel.out
+++ b/src/test/regress/expected/select_parallel.out
@@ -186,6 +186,12 @@ explain (costs off)
Index Cond: (hundred > 1)
(8 rows)
+select count((unique1)) from tenk1 where hundred > 1;
+ count
+-------
+ 9800
+(1 row)
+
reset enable_seqscan;
reset enable_indexscan;
-- test parallel merge join path.
diff --git a/src/test/regress/sql/select_parallel.sql b/src/test/regress/sql/select_parallel.sql
index 9311a77..e038a3a 100644
--- a/src/test/regress/sql/select_parallel.sql
+++ b/src/test/regress/sql/select_parallel.sql
@@ -70,6 +70,7 @@ set enable_indexscan to off;
explain (costs off)
select count((unique1)) from tenk1 where hundred > 1;
+select count((unique1)) from tenk1 where hundred > 1;
reset enable_seqscan;
reset enable_indexscan;
On 2017-04-01 17:23:04 +0530, Dilip Kumar wrote:
On Sat, Apr 1, 2017 at 12:16 AM, Andres Freund <andres@anarazel.de> wrote:
Hi,
The parallel code-path isn't actually exercised in the tests added in
[1], as evidenced by [2] (they just explain). That imo needs to be
fixed.Thanks for reporting. Attached patch fixes that.
That's better than before, but I'd appreciate working on a bit more
coverage. E.g. rescans probably aren't exercised in that test, right?
If you have time & energy, it'd also be good to expand the tests to
cover the prefetching logic - it's quite bad that it's currently not
tested at all :(
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On Mon, Apr 3, 2017 at 11:22 PM, Andres Freund <andres@anarazel.de> wrote:
That's better than before, but I'd appreciate working on a bit more
coverage. E.g. rescans probably aren't exercised in that test, right?If you have time & energy, it'd also be good to expand the tests to
cover the prefetching logic - it's quite bad that it's currently not
tested at all :(
Sure I can do that, In attached patch, I only fixed the problem of not
executing the bitmap test. Now, I will add few cases to cover other
parts especially rescan and prefetching logic.
--
Regards,
Dilip Kumar
EnterpriseDB: http://www.enterprisedb.com
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On Tue, Apr 4, 2017 at 5:51 AM, Dilip Kumar <dilipbalaut@gmail.com> wrote:
Sure I can do that, In attached patch, I only fixed the problem of not
executing the bitmap test. Now, I will add few cases to cover other
parts especially rescan and prefetching logic.
I have added two test cases to cover rescan, prefetch and lossy pages
logic for parallel bitmap. I have removed the existing case because
these two new cases will be enough to cover that part as well.
Now, nodeBitmapHeapScan.c has 95.5% of line coverage.
--
Regards,
Dilip Kumar
EnterpriseDB: http://www.enterprisedb.com
Attachments:
parallel_bitmap_test_v2.patchapplication/octet-stream; name=parallel_bitmap_test_v2.patchDownload
diff --git a/src/test/regress/expected/select_parallel.out b/src/test/regress/expected/select_parallel.out
index 038a62e..2463f1f 100644
--- a/src/test/regress/expected/select_parallel.out
+++ b/src/test/regress/expected/select_parallel.out
@@ -172,22 +172,50 @@ reset enable_bitmapscan;
-- test parallel bitmap heap scan.
set enable_seqscan to off;
set enable_indexscan to off;
+set enable_hashjoin to off;
+set enable_mergejoin to off;
+set enable_material to off;
+set effective_io_concurrency=50;
+set work_mem='64kB'; --set small work mem to force lossy pages
explain (costs off)
- select count((unique1)) from tenk1 where hundred > 1;
+ select count(*) from tenk1, tenk2 where tenk1.hundred > 1 and tenk2.thousand=0;
QUERY PLAN
------------------------------------------------------------
- Finalize Aggregate
- -> Gather
- Workers Planned: 4
- -> Partial Aggregate
+ Aggregate
+ -> Nested Loop
+ -> Seq Scan on tenk2
+ Filter: (thousand = 0)
+ -> Gather
+ Workers Planned: 4
-> Parallel Bitmap Heap Scan on tenk1
Recheck Cond: (hundred > 1)
-> Bitmap Index Scan on tenk1_hundred
Index Cond: (hundred > 1)
-(8 rows)
+(10 rows)
+
+select count(*) from tenk1, tenk2 where tenk1.hundred > 1 and tenk2.thousand=0;
+ count
+-------
+ 98000
+(1 row)
+
+create table bmscantest (a int, t text);
+insert into bmscantest select r, 'fooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooo' FROM generate_series(1,100000) r;
+create index i_bmtest ON bmscantest(a);
+select count(*) from bmscantest where a>1;
+ count
+-------
+ 99999
+(1 row)
reset enable_seqscan;
reset enable_indexscan;
+reset enable_hashjoin;
+reset enable_mergejoin;
+reset enable_material;
+reset effective_io_concurrency;
+reset work_mem;
+drop table bmscantest;
-- test parallel merge join path.
set enable_hashjoin to off;
set enable_nestloop to off;
diff --git a/src/test/regress/sql/select_parallel.sql b/src/test/regress/sql/select_parallel.sql
index 9311a77..53f4271 100644
--- a/src/test/regress/sql/select_parallel.sql
+++ b/src/test/regress/sql/select_parallel.sql
@@ -67,12 +67,28 @@ reset enable_bitmapscan;
-- test parallel bitmap heap scan.
set enable_seqscan to off;
set enable_indexscan to off;
-
+set enable_hashjoin to off;
+set enable_mergejoin to off;
+set enable_material to off;
+set effective_io_concurrency=50;
+set work_mem='64kB'; --set small work mem to force lossy pages
explain (costs off)
- select count((unique1)) from tenk1 where hundred > 1;
+ select count(*) from tenk1, tenk2 where tenk1.hundred > 1 and tenk2.thousand=0;
+select count(*) from tenk1, tenk2 where tenk1.hundred > 1 and tenk2.thousand=0;
+
+create table bmscantest (a int, t text);
+insert into bmscantest select r, 'fooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooo' FROM generate_series(1,100000) r;
+create index i_bmtest ON bmscantest(a);
+select count(*) from bmscantest where a>1;
reset enable_seqscan;
reset enable_indexscan;
+reset enable_hashjoin;
+reset enable_mergejoin;
+reset enable_material;
+reset effective_io_concurrency;
+reset work_mem;
+drop table bmscantest;
-- test parallel merge join path.
set enable_hashjoin to off;
On 2017-04-06 10:00:32 +0530, Dilip Kumar wrote:
On Tue, Apr 4, 2017 at 5:51 AM, Dilip Kumar <dilipbalaut@gmail.com> wrote:
Sure I can do that, In attached patch, I only fixed the problem of not
executing the bitmap test. Now, I will add few cases to cover other
parts especially rescan and prefetching logic.I have added two test cases to cover rescan, prefetch and lossy pages
logic for parallel bitmap. I have removed the existing case because
these two new cases will be enough to cover that part as well.Now, nodeBitmapHeapScan.c has 95.5% of line coverage.
Great! Pushed.
- Andres
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On 2017-04-06 13:43:55 -0700, Andres Freund wrote:
On 2017-04-06 10:00:32 +0530, Dilip Kumar wrote:
On Tue, Apr 4, 2017 at 5:51 AM, Dilip Kumar <dilipbalaut@gmail.com> wrote:
Sure I can do that, In attached patch, I only fixed the problem of not
executing the bitmap test. Now, I will add few cases to cover other
parts especially rescan and prefetching logic.I have added two test cases to cover rescan, prefetch and lossy pages
logic for parallel bitmap. I have removed the existing case because
these two new cases will be enough to cover that part as well.Now, nodeBitmapHeapScan.c has 95.5% of line coverage.
Great! Pushed.
At some point it might also be a good idea to compare parallel and
non-parallel results. It's obviously quite possible to break semantics
with parallelism...
- Andres
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers