Received: from malur.postgresql.org ([217.196.149.56]) by arkaria.postgresql.org with esmtp (Exim 4.84_2) (envelope-from ) id 1dou4P-0002VV-T0 for pgsql-hackers@arkaria.postgresql.org; Mon, 04 Sep 2017 16:17:54 +0000 Received: from localhost ([127.0.0.1] helo=postgresql.org) by malur.postgresql.org with smtp (Exim 4.84_2) (envelope-from ) id 1dou4P-0005jL-Fq for pgsql-hackers@arkaria.postgresql.org; Mon, 04 Sep 2017 16:17:53 +0000 Received: from magus.postgresql.org ([2a02:c0:301:0:ffff::29]) by malur.postgresql.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_CBC_SHA384:256) (Exim 4.84_2) (envelope-from ) id 1dou4O-0005eM-Af for pgsql-hackers@postgresql.org; Mon, 04 Sep 2017 16:17:52 +0000 Received: from mail.postgrespro.ru ([93.174.131.138]) by magus.postgresql.org with esmtp (Exim 4.84_2) (envelope-from ) id 1dou4G-0007r1-0e for pgsql-hackers@postgresql.org; Mon, 04 Sep 2017 16:17:51 +0000 Received: from localhost (localhost [127.0.0.1]) by mail.postgrespro.ru (Postfix) with ESMTP id CF06621C1D1D; Mon, 4 Sep 2017 19:17:42 +0300 (MSK) X-Virus-Scanned: Debian amavisd-new at postgrespro.ru Received: from [192.168.27.132] (unknown [192.168.27.1]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (Client did not present a certificate) by mail.postgrespro.ru (Postfix) with ESMTPSA id A319F21C1D06; Mon, 4 Sep 2017 19:17:42 +0300 (MSK) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=postgrespro.ru; s=mail; t=1504541862; bh=dVLNG4k3ICvN/6tOgmjJ1+a05XL8hJLgp+1YX4SFQCo=; h=Subject:To:References:From:Date:In-Reply-To; b=GG6oqBm+P5qSoNlemshALdOtttGZCJya1heYffLGyVWPitgX9Qo0LLxBXRhVjNvq4 arQu9vsRk9EjVGrN7KCA63uGMgbX1CMMqWKh+uSJu63GTIL5VMAefwVyEMdpauV1+R ojNTYSr6qOz72u3vxgwrQ8opzTmQmeuSfop4ewyM= Subject: Re: index-only count(*) for indexes supporting bitmap scans To: Alexey Chernyshov , pgsql-hackers@postgresql.org References: <239a8955-c0fc-f506-026d-c837e86c827b@postgrespro.ru> <122a32e4-73bd-ec85-9dad-a72729635c65@postgrespro.ru> <20170904121730.18925.18766.pgcf@coridan.postgresql.org> From: Alexander Kuzmenkov Message-ID: Date: Mon, 4 Sep 2017 19:17:42 +0300 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.2.1 MIME-Version: 1.0 In-Reply-To: <20170904121730.18925.18766.pgcf@coridan.postgresql.org> Content-Type: multipart/mixed; boundary="------------1C5E1408CBE2E8621DBABD91" Content-Language: en-US List-Archive: List-Help: List-ID: List-Owner: List-Post: List-Subscribe: List-Unsubscribe: X-Mailing-List: pgsql-hackers Precedence: bulk Sender: pgsql-hackers-owner@postgresql.org This is a multi-part message in MIME format. --------------1C5E1408CBE2E8621DBABD91 Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 7bit On 04.09.2017 15:17, Alexey Chernyshov wrote: > make check-world fails on contrib/postgres_fdw because of Subquery Scan on ... Probably, query plan was changed. Hi Alexey, Thanks for testing! This is the same problem as the one in 'select_distinct' I mentioned before. I changed the test, the updated patch is attached. -- Alexander Kuzmenkov Postgres Professional: http://www.postgrespro.com The Russian Postgres Company --------------1C5E1408CBE2E8621DBABD91 Content-Type: text/x-patch; name="index-only-count-v3.patch" Content-Transfer-Encoding: 7bit Content-Disposition: attachment; filename="index-only-count-v3.patch" diff --git a/contrib/postgres_fdw/expected/postgres_fdw.out b/contrib/postgres_fdw/expected/postgres_fdw.out index c19b3318c7..5f5f65d60c 100644 --- a/contrib/postgres_fdw/expected/postgres_fdw.out +++ b/contrib/postgres_fdw/expected/postgres_fdw.out @@ -2203,22 +2203,23 @@ SELECT t1c1, avg(t1c1 + t2c1) FROM (SELECT t1.c1, t2.c1 FROM ft1 t1 JOIN ft2 t2 -- join with lateral reference EXPLAIN (VERBOSE, COSTS OFF) SELECT t1."C 1" FROM "S 1"."T 1" t1, LATERAL (SELECT DISTINCT t2.c1, t3.c1 FROM ft1 t2, ft2 t3 WHERE t2.c1 = t3.c1 AND t2.c2 = t1.c2) q ORDER BY t1."C 1" OFFSET 10 LIMIT 10; - QUERY PLAN --------------------------------------------------------------------------------------------------------------------------------------------------------------------- + QUERY PLAN +-------------------------------------------------------------------------------------------------------------------------------------------------------------------------- Limit Output: t1."C 1" -> Nested Loop Output: t1."C 1" -> Index Scan using t1_pkey on "S 1"."T 1" t1 Output: t1."C 1", t1.c2, t1.c3, t1.c4, t1.c5, t1.c6, t1.c7, t1.c8 - -> HashAggregate - Output: t2.c1, t3.c1 - Group Key: t2.c1, t3.c1 - -> Foreign Scan + -> Subquery Scan on q + -> HashAggregate Output: t2.c1, t3.c1 - Relations: (public.ft1 t2) INNER JOIN (public.ft2 t3) - Remote SQL: SELECT r1."C 1", r2."C 1" FROM ("S 1"."T 1" r1 INNER JOIN "S 1"."T 1" r2 ON (((r1."C 1" = r2."C 1")) AND ((r1.c2 = $1::integer)))) -(13 rows) + Group Key: t2.c1, t3.c1 + -> Foreign Scan + Output: t2.c1, t3.c1 + Relations: (public.ft1 t2) INNER JOIN (public.ft2 t3) + Remote SQL: SELECT r1."C 1", r2."C 1" FROM ("S 1"."T 1" r1 INNER JOIN "S 1"."T 1" r2 ON (((r1."C 1" = r2."C 1")) AND ((r1.c2 = $1::integer)))) +(14 rows) SELECT t1."C 1" FROM "S 1"."T 1" t1, LATERAL (SELECT DISTINCT t2.c1, t3.c1 FROM ft1 t2, ft2 t3 WHERE t2.c1 = t3.c1 AND t2.c2 = t1.c2) q ORDER BY t1."C 1" OFFSET 10 LIMIT 10; C 1 @@ -2672,16 +2673,17 @@ select c2, sum(c1) from ft2 group by c2 having avg(c1) < 500 and sum(c1) < 49800 -- Unshippable HAVING clause will be evaluated locally, and other qual in HAVING clause is pushed down explain (verbose, costs off) select count(*) from (select c5, count(c1) from ft1 group by c5, sqrt(c2) having (avg(c1) / avg(c1)) * random() <= 1 and avg(c1) < 500) x; - QUERY PLAN -------------------------------------------------------------------------------------------------------------------------------------------------- + QUERY PLAN +------------------------------------------------------------------------------------------------------------------------------------------------------- Aggregate Output: count(*) - -> Foreign Scan - Output: ft1.c5, NULL::bigint, (sqrt((ft1.c2)::double precision)) - Filter: (((((avg(ft1.c1)) / (avg(ft1.c1))))::double precision * random()) <= '1'::double precision) - Relations: Aggregate on (public.ft1) - Remote SQL: SELECT c5, NULL::bigint, sqrt(c2), avg("C 1") FROM "S 1"."T 1" GROUP BY c5, (sqrt(c2)) HAVING ((avg("C 1") < 500::numeric)) -(7 rows) + -> Subquery Scan on x + -> Foreign Scan + Output: ft1.c5, NULL::bigint, (sqrt((ft1.c2)::double precision)) + Filter: (((((avg(ft1.c1)) / (avg(ft1.c1))))::double precision * random()) <= '1'::double precision) + Relations: Aggregate on (public.ft1) + Remote SQL: SELECT c5, NULL::bigint, sqrt(c2), avg("C 1") FROM "S 1"."T 1" GROUP BY c5, (sqrt(c2)) HAVING ((avg("C 1") < 500::numeric)) +(8 rows) select count(*) from (select c5, count(c1) from ft1 group by c5, sqrt(c2) having (avg(c1) / avg(c1)) * random() <= 1 and avg(c1) < 500) x; count diff --git a/src/backend/executor/nodeBitmapHeapscan.c b/src/backend/executor/nodeBitmapHeapscan.c index 79f534e4e9..d7ea6f6929 100644 --- a/src/backend/executor/nodeBitmapHeapscan.c +++ b/src/backend/executor/nodeBitmapHeapscan.c @@ -39,6 +39,7 @@ #include "access/relscan.h" #include "access/transam.h" +#include "access/visibilitymap.h" #include "executor/execdebug.h" #include "executor/nodeBitmapHeapscan.h" #include "miscadmin.h" @@ -225,9 +226,25 @@ BitmapHeapNext(BitmapHeapScanState *node) } /* - * Fetch the current heap page and identify candidate tuples. + * If we don't need the tuple contents and are only counting them, + * we can skip fetching the page if the bitmap doesn't need rechecking + * and all tuples on the page are visible to our transaction */ - bitgetpage(scan, tbmres); + node->bhs_nofetch = !tbmres->recheck + && node->ss.ps.qual == NULL + && node->ss.ps.plan->targetlist == NIL + && VM_ALL_VISIBLE(node->ss.ss_currentRelation, tbmres->blockno, + &node->bhs_vmbuffer); + + if (node->bhs_nofetch) + scan->rs_ntuples = tbmres->ntuples; + else + { + /* + * Fetch the current heap page and identify candidate tuples. + */ + bitgetpage(scan, tbmres); + } if (tbmres->ntuples >= 0) node->exact_pages++; @@ -289,45 +306,58 @@ BitmapHeapNext(BitmapHeapScanState *node) */ BitmapPrefetch(node, scan); - /* - * Okay to fetch the tuple - */ - targoffset = scan->rs_vistuples[scan->rs_cindex]; - dp = (Page) BufferGetPage(scan->rs_cbuf); - lp = PageGetItemId(dp, targoffset); - Assert(ItemIdIsNormal(lp)); - scan->rs_ctup.t_data = (HeapTupleHeader) PageGetItem((Page) dp, lp); - scan->rs_ctup.t_len = ItemIdGetLength(lp); - scan->rs_ctup.t_tableOid = scan->rs_rd->rd_id; - ItemPointerSet(&scan->rs_ctup.t_self, tbmres->blockno, targoffset); + if (node->bhs_nofetch) + { + /* + * If we don't have to fetch the tuple, just return a + * bogus one + */ + slot->tts_isempty = false; + slot->tts_nvalid = 0; + } + else + { + /* + * Okay to fetch the tuple + */ + targoffset = scan->rs_vistuples[scan->rs_cindex]; + dp = (Page) BufferGetPage(scan->rs_cbuf); + lp = PageGetItemId(dp, targoffset); + Assert(ItemIdIsNormal(lp)); - pgstat_count_heap_fetch(scan->rs_rd); + scan->rs_ctup.t_data = (HeapTupleHeader) PageGetItem((Page) dp, lp); + scan->rs_ctup.t_len = ItemIdGetLength(lp); + scan->rs_ctup.t_tableOid = scan->rs_rd->rd_id; + ItemPointerSet(&scan->rs_ctup.t_self, tbmres->blockno, targoffset); - /* - * Set up the result slot to point to this tuple. Note that the slot - * acquires a pin on the buffer. - */ - ExecStoreTuple(&scan->rs_ctup, - slot, - scan->rs_cbuf, - false); + pgstat_count_heap_fetch(scan->rs_rd); - /* - * If we are using lossy info, we have to recheck the qual conditions - * at every tuple. - */ - if (tbmres->recheck) - { - econtext->ecxt_scantuple = slot; - ResetExprContext(econtext); + /* + * Set up the result slot to point to this tuple. Note that the slot + * acquires a pin on the buffer. + */ + ExecStoreTuple(&scan->rs_ctup, + slot, + scan->rs_cbuf, + false); - if (!ExecQual(node->bitmapqualorig, econtext)) + /* + * If we are using lossy info, we have to recheck the qual conditions + * at every tuple. + */ + if (tbmres->recheck) { - /* Fails recheck, so drop it and loop back for another */ - InstrCountFiltered2(node, 1); - ExecClearTuple(slot); - continue; + econtext->ecxt_scantuple = slot; + ResetExprContext(econtext); + + if (!ExecQual(node->bitmapqualorig, econtext)) + { + /* Fails recheck, so drop it and loop back for another */ + InstrCountFiltered2(node, 1); + ExecClearTuple(slot); + continue; + } } } @@ -573,6 +603,15 @@ BitmapPrefetch(BitmapHeapScanState *node, HeapScanDesc scan) #ifdef USE_PREFETCH ParallelBitmapHeapState *pstate = node->pstate; + if (node->bhs_nofetch) + { + /* + * If we did not need to fetch the current page, + * we probably will not need to fetch the next. + */ + return; + } + if (pstate == NULL) { TBMIterator *prefetch_iterator = node->prefetch_iterator; @@ -778,6 +817,11 @@ ExecEndBitmapHeapScan(BitmapHeapScanState *node) if (node->shared_prefetch_iterator) tbm_end_shared_iterate(node->shared_prefetch_iterator); + if (node->bhs_vmbuffer != InvalidBuffer) + { + ReleaseBuffer(node->bhs_vmbuffer); + } + /* * close heap scan */ @@ -827,6 +871,8 @@ ExecInitBitmapHeapScan(BitmapHeapScan *node, EState *estate, int eflags) scanstate->prefetch_iterator = NULL; scanstate->prefetch_pages = 0; scanstate->prefetch_target = 0; + scanstate->bhs_vmbuffer = InvalidBuffer; + scanstate->bhs_nofetch = false; /* may be updated below */ scanstate->prefetch_maximum = target_prefetch_pages; scanstate->pscan_len = 0; diff --git a/src/backend/optimizer/path/costsize.c b/src/backend/optimizer/path/costsize.c index b35acb7bdc..13fb8d3153 100644 --- a/src/backend/optimizer/path/costsize.c +++ b/src/backend/optimizer/path/costsize.c @@ -982,6 +982,14 @@ cost_bitmap_heap_scan(Path *path, PlannerInfo *root, RelOptInfo *baserel, else cost_per_page = spc_random_page_cost; + /* + * Bitmap heap scan tries not to fetch the heap pages if the targetlist + * is empty. It should be reflected in the cost somehow; for now, just + * halve the number of pages to be fetched. + */ + if (path->pathtarget->exprs == NIL) + pages_fetched = pages_fetched / 2; + run_cost += pages_fetched * cost_per_page; /* diff --git a/src/backend/optimizer/plan/createplan.c b/src/backend/optimizer/plan/createplan.c index 5c934f223d..0874871b61 100644 --- a/src/backend/optimizer/plan/createplan.c +++ b/src/backend/optimizer/plan/createplan.c @@ -778,6 +778,13 @@ use_physical_tlist(PlannerInfo *root, Path *path, int flags) return false; /* + * If the tlist is empty, keep it as is, so that the + * bitmap heap scan could try skipping heap page fetches. + */ + if (path->pathtarget->exprs == NIL) + return false; + + /* * We can do this for real relation scans, subquery scans, function scans, * tablefunc scans, values scans, and CTE scans (but not for, eg, joins). */ diff --git a/src/include/nodes/execnodes.h b/src/include/nodes/execnodes.h index 35c28a6143..6f567f25e8 100644 --- a/src/include/nodes/execnodes.h +++ b/src/include/nodes/execnodes.h @@ -1334,6 +1334,8 @@ typedef struct ParallelBitmapHeapState * shared_tbmiterator shared iterator * shared_prefetch_iterator shared iterator for prefetching * pstate shared state for parallel bitmap scan + * bhs_vmbuffer buffer for visibility map lookups + * bhs_nofetch can we not fetch the heap page and return an empty tuple? * ---------------- */ typedef struct BitmapHeapScanState @@ -1354,6 +1356,8 @@ typedef struct BitmapHeapScanState TBMSharedIterator *shared_tbmiterator; TBMSharedIterator *shared_prefetch_iterator; ParallelBitmapHeapState *pstate; + Buffer bhs_vmbuffer; + bool bhs_nofetch; } BitmapHeapScanState; /* ---------------- diff --git a/src/test/regress/expected/join.out b/src/test/regress/expected/join.out index 9f4c88dab4..2e82ab1ec0 100644 --- a/src/test/regress/expected/join.out +++ b/src/test/regress/expected/join.out @@ -3746,7 +3746,6 @@ where tt1.f1 = ss1.c0; Output: tt1.f1 Filter: (tt1.f1 = 'foo'::text) -> Seq Scan on public.text_tbl tt2 - Output: tt2.f1 -> Materialize Output: tt4.f1 -> Nested Loop Left Join @@ -3765,7 +3764,7 @@ where tt1.f1 = ss1.c0; Output: (tt4.f1) -> Seq Scan on public.text_tbl tt5 Output: tt4.f1 -(29 rows) +(28 rows) select 1 from text_tbl as tt1 diff --git a/src/test/regress/expected/select_distinct.out b/src/test/regress/expected/select_distinct.out index f3696c6d1d..d29c1adaab 100644 --- a/src/test/regress/expected/select_distinct.out +++ b/src/test/regress/expected/select_distinct.out @@ -130,16 +130,17 @@ SELECT DISTINCT p.age FROM person* p ORDER BY age using >; EXPLAIN (VERBOSE, COSTS OFF) SELECT count(*) FROM (SELECT DISTINCT two, four, two FROM tenk1) ss; - QUERY PLAN --------------------------------------------------------- + QUERY PLAN +-------------------------------------------------------------- Aggregate Output: count(*) - -> HashAggregate - Output: tenk1.two, tenk1.four, tenk1.two - Group Key: tenk1.two, tenk1.four, tenk1.two - -> Seq Scan on public.tenk1 + -> Subquery Scan on ss + -> HashAggregate Output: tenk1.two, tenk1.four, tenk1.two -(7 rows) + Group Key: tenk1.two, tenk1.four, tenk1.two + -> Seq Scan on public.tenk1 + Output: tenk1.two, tenk1.four, tenk1.two +(8 rows) SELECT count(*) FROM (SELECT DISTINCT two, four, two FROM tenk1) ss; diff --git a/src/test/regress/expected/stats.out b/src/test/regress/expected/stats.out index fc91f3ce36..4c4ee4d808 100644 --- a/src/test/regress/expected/stats.out +++ b/src/test/regress/expected/stats.out @@ -136,12 +136,15 @@ SELECT count(*) FROM tenk2; (1 row) -- do an indexscan +-- disable bitmap scan because it can skip fetching the heap tuples +SET enable_bitmapscan to OFF; SELECT count(*) FROM tenk2 WHERE unique1 = 1; count ------- 1 (1 row) +SET enable_bitmapscan to ON; -- We can't just call wait_for_stats() at this point, because we only -- transmit stats when the session goes idle, and we probably didn't -- transmit the last couple of counts yet thanks to the rate-limiting logic diff --git a/src/test/regress/expected/updatable_views.out b/src/test/regress/expected/updatable_views.out index 2090a411fe..fc1b512a35 100644 --- a/src/test/regress/expected/updatable_views.out +++ b/src/test/regress/expected/updatable_views.out @@ -1658,20 +1658,22 @@ UPDATE rw_view1 SET a = a + 5; -- should fail ERROR: new row violates check option for view "rw_view1" DETAIL: Failing row contains (15). EXPLAIN (costs off) INSERT INTO rw_view1 VALUES (5); - QUERY PLAN ---------------------------------------------------------------- + QUERY PLAN +------------------------------------------------------- Insert on base_tbl b -> Result SubPlan 1 - -> Index Only Scan using ref_tbl_pkey on ref_tbl r - Index Cond: (a = b.a) + -> Bitmap Heap Scan on ref_tbl r + Recheck Cond: (a = b.a) + -> Bitmap Index Scan on ref_tbl_pkey + Index Cond: (a = b.a) SubPlan 2 -> Seq Scan on ref_tbl r_1 -(7 rows) +(9 rows) EXPLAIN (costs off) UPDATE rw_view1 SET a = a + 5; - QUERY PLAN ------------------------------------------------------------------ + QUERY PLAN +------------------------------------------------------- Update on base_tbl b -> Hash Join Hash Cond: (b.a = r.a) @@ -1679,11 +1681,13 @@ EXPLAIN (costs off) UPDATE rw_view1 SET a = a + 5; -> Hash -> Seq Scan on ref_tbl r SubPlan 1 - -> Index Only Scan using ref_tbl_pkey on ref_tbl r_1 - Index Cond: (a = b.a) + -> Bitmap Heap Scan on ref_tbl r_1 + Recheck Cond: (a = b.a) + -> Bitmap Index Scan on ref_tbl_pkey + Index Cond: (a = b.a) SubPlan 2 -> Seq Scan on ref_tbl r_2 -(11 rows) +(13 rows) DROP TABLE base_tbl, ref_tbl CASCADE; NOTICE: drop cascades to view rw_view1 @@ -2024,24 +2028,28 @@ EXPLAIN (costs off) DELETE FROM rw_view1 WHERE id = 1 AND snoop(data); DELETE FROM rw_view1 WHERE id = 1 AND snoop(data); NOTICE: snooped value: Row 1 EXPLAIN (costs off) INSERT INTO rw_view1 VALUES (2, 'New row 2'); - QUERY PLAN ------------------------------------------------------------ + QUERY PLAN +-------------------------------------------------------- Insert on base_tbl InitPlan 1 (returns $0) - -> Index Only Scan using base_tbl_pkey on base_tbl t - Index Cond: (id = 2) + -> Bitmap Heap Scan on base_tbl t + Recheck Cond: (id = 2) + -> Bitmap Index Scan on base_tbl_pkey + Index Cond: (id = 2) -> Result One-Time Filter: ($0 IS NOT TRUE) Update on base_tbl InitPlan 1 (returns $0) - -> Index Only Scan using base_tbl_pkey on base_tbl t - Index Cond: (id = 2) + -> Bitmap Heap Scan on base_tbl t + Recheck Cond: (id = 2) + -> Bitmap Index Scan on base_tbl_pkey + Index Cond: (id = 2) -> Result One-Time Filter: $0 -> Index Scan using base_tbl_pkey on base_tbl Index Cond: (id = 2) -(15 rows) +(19 rows) INSERT INTO rw_view1 VALUES (2, 'New row 2'); SELECT * FROM base_tbl; diff --git a/src/test/regress/sql/stats.sql b/src/test/regress/sql/stats.sql index 6e882bf3ac..cec32cff2d 100644 --- a/src/test/regress/sql/stats.sql +++ b/src/test/regress/sql/stats.sql @@ -138,7 +138,10 @@ ROLLBACK; -- do a seqscan SELECT count(*) FROM tenk2; -- do an indexscan +-- disable bitmap scan because it can skip fetching the heap tuples +SET enable_bitmapscan to OFF; SELECT count(*) FROM tenk2 WHERE unique1 = 1; +SET enable_bitmapscan to ON; -- We can't just call wait_for_stats() at this point, because we only -- transmit stats when the session goes idle, and we probably didn't --------------1C5E1408CBE2E8621DBABD91 Content-Type: text/plain Content-Disposition: inline Content-Transfer-Encoding: 8bit MIME-Version: 1.0 -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers --------------1C5E1408CBE2E8621DBABD91--