Re: [HACKERS] parallel index(-only) scan breaks when run without parallelism

[Amit Kapila]

On Wed, Mar 8, 2017 at 12:27 AM, Robert Haas  wrote:
> Amit, Rafia,
>
> nodeIndexscan.c, unlike nodeSeqscan.c, thinks that a parallel-aware
> scan will always be executed in parallel mode.  But that's not true:
> an Execute message with a non-zero row count could cause us to abandon
> planned parallelism and execute the plan serially.
>

Right, and the current code had assumed that if there is a parallel
plan then it will always enter the parallel mode.   I think the fix is
quite similar to what we do in nodeSeqscan.c i.e. initialize the scan
descriptor before starting the scan if it is not already initialized.
 There is an additional check required for ensuring if index runtime
keys are ready before calling index_rescan.  Attached patch fixes the
problem.


-- 
With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com


fix_scandesc_initialization_parallel_index_v1.patch
Description: Binary data

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

[HACKERS] parallel index(-only) scan breaks when run without parallelism

[Robert Haas]

Amit, Rafia,

nodeIndexscan.c, unlike nodeSeqscan.c, thinks that a parallel-aware
scan will always be executed in parallel mode.  But that's not true:
an Execute message with a non-zero row count could cause us to abandon
planned parallelism and execute the plan serially.   I believe this
would cause a core dump.  We definitely core dump with the following
small patch, which causes parallelism to always be abandoned:

diff --git a/src/backend/executor/execMain.c b/src/backend/executor/execMain.c
index f5cd65d..fc4de48 100644
--- a/src/backend/executor/execMain.c
+++ b/src/backend/executor/execMain.c
@@ -1596,8 +1596,7 @@ ExecutePlan(EState *estate,
  * when writing into a relation, because no database changes are allowed
  * in parallel mode.
  */
-if (numberTuples || dest->mydest == DestIntoRel)
-use_parallel_mode = false;
+use_parallel_mode = false;

 if (use_parallel_mode)
 EnterParallelMode();

I believe this defect was introduced by
5262f7a4fc44f651241d2ff1fa688dd664a34874 and that nodeIndexonlyscan.c
has the same defect as of 0414b26bac09379a4cbf1fbd847d1cee2293c5e4.

Please fix.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] Parallel Index-only scan

[Rafia Sabih]

On Sun, Feb 19, 2017 at 4:02 PM, Robert Haas  wrote:

> Committed, although I neglected to incorporate this change.  Not sure
> if I should go back and do that; it doesn't read too badly as-is.
>
Thanks Robert for committing, and thanks Rahila, Amit, and Tushar for
reviewing and testing the patch.

-- 
Regards,
Rafia Sabih
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

Re: [HACKERS] Parallel Index-only scan

[Robert Haas]

On Sat, Feb 18, 2017 at 12:02 PM, Amit Kapila  wrote:
> +   /*
> +* If we are here to just update the scan keys, then don't
> reset parallel
> +* scan. For detailed reason behind this look in the comments for
> +* ExecReScanIndexScan.
> +*/
>
> You can phrase the second line as "See ExecReScanIndexScan for
> details.".  Apart from that this patch looks good to me.  I have
> marked this patch as "Ready For Committer".

Committed, although I neglected to incorporate this change.  Not sure
if I should go back and do that; it doesn't read too badly as-is.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] Parallel Index-only scan

[Amit Kapila]

On Fri, Feb 17, 2017 at 10:35 AM, Rafia Sabih
 wrote:
>
>
> On Thu, Feb 16, 2017 at 9:25 PM, Amit Kapila 
> wrote:
>>
>>
>> 4.
>> ExecReScanIndexOnlyScan(IndexOnlyScanState *node)
>> {
>> ..
>> + /*
>> + * if we are here to just update the scan keys, then don't reset parallel
>> + * scan
>> + */
>> + if (node->ioss_NumRuntimeKeys != 0 && !node->ioss_RuntimeKeysReady)
>> + reset_parallel_scan = false;
>> ..
>> }
>>
>> I think here you can update the comment to indicate that for detailed
>> reason refer ExecReScanIndexScan.
>
>
> Done.

+   /*
+* If we are here to just update the scan keys, then don't
reset parallel
+* scan. For detailed reason behind this look in the comments for
+* ExecReScanIndexScan.
+*/

You can phrase the second line as "See ExecReScanIndexScan for
details.".  Apart from that this patch looks good to me.  I have
marked this patch as "Ready For Committer".

-- 
With Regards,
Amit Kapila.
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

Re: [HACKERS] Parallel Index-only scan

[Rafia Sabih]

On Thu, Feb 16, 2017 at 9:25 PM, Amit Kapila 
wrote:
>
> Few comments:
>
> 1.
> + * ioss_PscanLen   This is needed for parallel index scan
>   * 
>   */
>  typedef struct IndexOnlyScanState
> @@ -1427,6 +1428,7 @@ typedef struct IndexOnlyScanState
>   IndexScanDesc ioss_ScanDesc;
>   Buffer ioss_VMBuffer;
>   long ioss_HeapFetches;
> + Size ioss_PscanLen; /* This is needed for parallel index scan */
>
> No need to mention same comment at multiple places.  I think you keep
> it on top of structure.  The explanation could be some thing like
> "size of parallel index only scan descriptor"
>

Fixed.

>
> 2.
> + node->ioss_ScanDesc->xs_want_itup = true;
>
> I think wherever you are initializing xs_want_itup, you should
> initialize ioss_VMBuffer as well.  Is there a reason for not doing so?
>

Done.

>
>
> 3.
> explain (costs off)
>   select  sum(parallel_restricted(unique1)) from tenk1
>   group by(parallel_restricted(unique1));
> - QUERY PLAN
> -
> +QUERY PLAN
> +---
>   HashAggregate
> Group Key: parallel_restricted(unique1)
> -   ->  Index Only Scan using tenk1_unique1 on tenk1
> -(3 rows)
> +   ->  Gather
> + Workers Planned: 4
> + ->  Parallel Index Only Scan using tenk1_unique1 on tenk1
> +(5 rows)
>
> It doesn't look good that you want to test parallel index only scan
> for a test case that wants to test restricted function.  The comments
> atop test looks odd. I suggest add a separate test (both explain and
> actual execution of query) for parallel index only scan as we have for
> parallel plans for other queries and see if we can keep the output of
> existing test same.
>

Agree, but actually the objective of this test-case is met even with this
plan. To restrict parallel index-only scan here, modification in query or
other parameters would be required. However, for the proper code-coverage
and otherwise I have added test-case for parallel index-only scan.

>
> 4.
> ExecReScanIndexOnlyScan(IndexOnlyScanState *node)
> {
> ..
> + /*
> + * if we are here to just update the scan keys, then don't reset parallel
> + * scan
> + */
> + if (node->ioss_NumRuntimeKeys != 0 && !node->ioss_RuntimeKeysReady)
> + reset_parallel_scan = false;
> ..
> }
>
> I think here you can update the comment to indicate that for detailed
> reason refer ExecReScanIndexScan.
>

Done.
Please find the attached patch for the revised version.
Just an FYI, in my recent tests on TPC-H 300 scale factor, Q16 showed
improved execution time from 830 seconds to 730 seconds with this patch
when used with parallel merge-join patch [1].

[1]
https://www.postgresql.org/message-id/CAFiTN-tX3EzDw7zYvi97eNADG9PH-nmhLa24Y3uWdzy_Y4SkfQ%40mail.gmail.com
Regards,
Rafia Sabih
EnterpriseDB: http://www.enterprisedb.com/


parallel_index_only_v8.patch
Description: Binary data

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] Parallel Index-only scan

[Amit Kapila]

On Thu, Feb 16, 2017 at 3:57 PM, Rafia Sabih
 wrote:
>
>
> On Thu, Feb 16, 2017 at 3:40 PM, Rafia Sabih 
> wrote:
>>
>>
>> Please find the attached patch for rebased and cleaner version.
>>
> Please find the attached patch with a minor comment update.
>

Few comments:

1.
+ * ioss_PscanLen   This is needed for parallel index scan
  * 
  */
 typedef struct IndexOnlyScanState
@@ -1427,6 +1428,7 @@ typedef struct IndexOnlyScanState
  IndexScanDesc ioss_ScanDesc;
  Buffer ioss_VMBuffer;
  long ioss_HeapFetches;
+ Size ioss_PscanLen; /* This is needed for parallel index scan */

No need to mention same comment at multiple places.  I think you keep
it on top of structure.  The explanation could be some thing like
"size of parallel index only scan descriptor"

2.
+ node->ioss_ScanDesc->xs_want_itup = true;

I think wherever you are initializing xs_want_itup, you should
initialize ioss_VMBuffer as well.  Is there a reason for not doing so?


3.
explain (costs off)
  select  sum(parallel_restricted(unique1)) from tenk1
  group by(parallel_restricted(unique1));
- QUERY PLAN
-
+QUERY PLAN
+---
  HashAggregate
Group Key: parallel_restricted(unique1)
-   ->  Index Only Scan using tenk1_unique1 on tenk1
-(3 rows)
+   ->  Gather
+ Workers Planned: 4
+ ->  Parallel Index Only Scan using tenk1_unique1 on tenk1
+(5 rows)

It doesn't look good that you want to test parallel index only scan
for a test case that wants to test restricted function.  The comments
atop test looks odd. I suggest add a separate test (both explain and
actual execution of query) for parallel index only scan as we have for
parallel plans for other queries and see if we can keep the output of
existing test same.

4.
ExecReScanIndexOnlyScan(IndexOnlyScanState *node)
{
..
+ /*
+ * if we are here to just update the scan keys, then don't reset parallel
+ * scan
+ */
+ if (node->ioss_NumRuntimeKeys != 0 && !node->ioss_RuntimeKeysReady)
+ reset_parallel_scan = false;
..
}

I think here you can update the comment to indicate that for detailed
reason refer ExecReScanIndexScan.



-- 
With Regards,
Amit Kapila.
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

Re: [HACKERS] Parallel Index-only scan

[Rafia Sabih]

On Thu, Feb 16, 2017 at 3:40 PM, Rafia Sabih 
wrote:

>
> On Thu, Feb 16, 2017 at 1:26 PM, Rahila Syed 
> wrote:
>
>> I  reviewed the patch. Overall it looks fine to me.
>>
>> One comment,
>>
>> >-   if (index->amcanparallel &&
>> >-   !index_only_scan &&
>> >+   if ((index->amcanparallel ||
>> >+   index_only_scan) &&
>>
>> Why do we need to check for index_only_scan in the above condition. IIUC,
>> index->amcanparallel is necessary for
>> parallel index scan to be chosen. We cannot chose parallel index only
>> scan if index_only_scan is happening
>> without worrying about index->amcanparallel value. So OR-ing
>> index->amcanparallel with index_only_scan is probably not
>> correct.
>>
>> True, we do not need this, only removing !index_only_scan should work.
> Fixed
>
>>
>>
>> On Thu, Feb 16, 2017 at 1:06 AM, Robert Haas 
>> wrote:
>>>
>>>
>>> This again needs minor rebasing but basically looks fine.  It's a
>>> pretty straightforward extension of the parallel index scan work.
>>>
>>> Please make sure that this is pgindent-clean - i.e. that when you
>>> pgindent the files that it touches, pgindent doesn't change anything
>>> of the same parts of the file that you've changed in the patch.  Also,
>>> I believe Amit may have made some adjustments to the logic in
>>> nodeIndexScan.c; if so, it would be good to make sure that the
>>> nodeIndexOnlyScan.c changes match what was done there.  In particular,
>>> he's got this:
>>>
>>> if (reset_parallel_scan && node->iss_ScanDesc->parallel_s
>>> can)
>>> index_parallelrescan(node->iss_ScanDesc);
>>>
>>> And you've got this:
>>>
>>> +   if (reset_parallel_scan)
>>> +   index_parallelrescan(node->ioss_ScanDesc);
>>>
>>
> Fixed.
> Please find the attached patch for rebased and cleaner version.
>
> Please find the attached patch with a minor comment update.

-- 
Regards,
Rafia Sabih
EnterpriseDB: http://www.enterprisedb.com/


parallel_index_only_v7.patch
Description: Binary data

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] Parallel Index-only scan

[Rafia Sabih]

On Thu, Feb 16, 2017 at 1:26 PM, Rahila Syed  wrote:

> I  reviewed the patch. Overall it looks fine to me.
>
> One comment,
>
> >-   if (index->amcanparallel &&
> >-   !index_only_scan &&
> >+   if ((index->amcanparallel ||
> >+   index_only_scan) &&
>
> Why do we need to check for index_only_scan in the above condition. IIUC,
> index->amcanparallel is necessary for
> parallel index scan to be chosen. We cannot chose parallel index only scan
> if index_only_scan is happening
> without worrying about index->amcanparallel value. So OR-ing
> index->amcanparallel with index_only_scan is probably not
> correct.
>
> True, we do not need this, only removing !index_only_scan should work.
Fixed

>
>
> On Thu, Feb 16, 2017 at 1:06 AM, Robert Haas 
> wrote:
>>
>>
>> This again needs minor rebasing but basically looks fine.  It's a
>> pretty straightforward extension of the parallel index scan work.
>>
>> Please make sure that this is pgindent-clean - i.e. that when you
>> pgindent the files that it touches, pgindent doesn't change anything
>> of the same parts of the file that you've changed in the patch.  Also,
>> I believe Amit may have made some adjustments to the logic in
>> nodeIndexScan.c; if so, it would be good to make sure that the
>> nodeIndexOnlyScan.c changes match what was done there.  In particular,
>> he's got this:
>>
>> if (reset_parallel_scan && node->iss_ScanDesc->parallel_s
>> can)
>> index_parallelrescan(node->iss_ScanDesc);
>>
>> And you've got this:
>>
>> +   if (reset_parallel_scan)
>> +   index_parallelrescan(node->ioss_ScanDesc);
>>
>
Fixed.
Please find the attached patch for rebased and cleaner version.

-- 
Regards,
Rafia Sabih
EnterpriseDB: http://www.enterprisedb.com/


parallel_index_only_v6.patch
Description: Binary data

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] Parallel Index-only scan

[Rahila Syed]

I  reviewed the patch. Overall it looks fine to me.

One comment,

>-   if (index->amcanparallel &&
>-   !index_only_scan &&
>+   if ((index->amcanparallel ||
>+   index_only_scan) &&

Why do we need to check for index_only_scan in the above condition. IIUC,
index->amcanparallel is necessary for
parallel index scan to be chosen. We cannot chose parallel index only scan
if index_only_scan is happening
without worrying about index->amcanparallel value. So OR-ing
index->amcanparallel with index_only_scan is probably not
correct.

Thank you,
Rahila Syed




On Thu, Feb 16, 2017 at 1:06 AM, Robert Haas  wrote:

> On Thu, Jan 19, 2017 at 7:07 AM, Rafia Sabih
>  wrote:
> > Please find the attached file rebased patch of parallel index-only
> > scan on the latest Parallel index-scan patch [1].
>
> This again needs minor rebasing but basically looks fine.  It's a
> pretty straightforward extension of the parallel index scan work.
>
> Please make sure that this is pgindent-clean - i.e. that when you
> pgindent the files that it touches, pgindent doesn't change anything
> of the same parts of the file that you've changed in the patch.  Also,
> I believe Amit may have made some adjustments to the logic in
> nodeIndexScan.c; if so, it would be good to make sure that the
> nodeIndexOnlyScan.c changes match what was done there.  In particular,
> he's got this:
>
> if (reset_parallel_scan && node->iss_ScanDesc->parallel_
> scan)
> index_parallelrescan(node->iss_ScanDesc);
>
> And you've got this:
>
> +   if (reset_parallel_scan)
> +   index_parallelrescan(node->ioss_ScanDesc);
>
> There might be some other inconsistencies as well that I didn't notice
> on a quick look.
>
> --
> Robert Haas
> EnterpriseDB: http://www.enterprisedb.com
> The Enterprise PostgreSQL Company
>

Re: [HACKERS] Parallel Index-only scan

[Robert Haas]

On Thu, Jan 19, 2017 at 7:07 AM, Rafia Sabih
 wrote:
> Please find the attached file rebased patch of parallel index-only
> scan on the latest Parallel index-scan patch [1].

This again needs minor rebasing but basically looks fine.  It's a
pretty straightforward extension of the parallel index scan work.

Please make sure that this is pgindent-clean - i.e. that when you
pgindent the files that it touches, pgindent doesn't change anything
of the same parts of the file that you've changed in the patch.  Also,
I believe Amit may have made some adjustments to the logic in
nodeIndexScan.c; if so, it would be good to make sure that the
nodeIndexOnlyScan.c changes match what was done there.  In particular,
he's got this:

if (reset_parallel_scan && node->iss_ScanDesc->parallel_scan)
index_parallelrescan(node->iss_ScanDesc);

And you've got this:

+   if (reset_parallel_scan)
+   index_parallelrescan(node->ioss_ScanDesc);

There might be some other inconsistencies as well that I didn't notice
on a quick look.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] Parallel Index-only scan

[Michael Paquier]

On Thu, Jan 19, 2017 at 9:07 PM, Rafia Sabih
 wrote:
> Please find the attached file rebased patch of parallel index-only
> scan on the latest Parallel index-scan patch [1].

Moved to CF 2017-03.
-- 
Michael


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] Parallel Index-only scan

[tushar]

On 01/19/2017 05:37 PM, Rafia Sabih wrote:

Please find the attached file rebased patch of parallel index-only
scan on the latest Parallel index-scan patch [1].
We did some testing of  this feature and written few testcases.  PFA the 
sql script(along with the expected .out files)


In addition we have generated the LCOV (code coverage) report and 
compared the files which are changed for this.
You can see the numbers for  "with_patch" V/s "with_patch+TestCases"  
(.pdf file is attached)


--
regards,tushar
EnterpriseDB  https://www.enterprisedb.com/
The Enterprise PostgreSQL Company

--create database
--create database test;
-- Running script for pgbanch tables against a scaling factor of 1,000 
--\! ./pgbench -i -s 1000 test > /tmp/my_logs 2>&1
--\c test
-- TestCase: 1
--Description 
/*
Explain plan catching Parallel index Only scan when in WHERE condition
   a) Single column of integer Type having index.
   b) condition: where the column having index is trying to fetch NOT NULL values("IS NOT NULL").
-- Need to disable SEQUENTIAL SCAN  to reproduce explain plan catch "parallel index Only scan"
*/
\c test
set enable_seqscan =0;
explain analyze  verbose select count(*) from pgbench_accounts where aid is not null;
-- TestCase: 2
--Description 
/*
Explain plan catching  parallel index Only scan:
   a) both columns are of integer Type.
   b) 1 column is having index and another is non key column.
   c) condition: where the column having BETWEEN .. AND .. condition both in Index and Non-Index column in WHERE condition.
*/
explain analyze verbose  select count(aid) from pgbench_accounts where aid between 1000 and 9000 ;

-- TestCase: 3
--Description 
/*
Explain plan catching  parallel index Only scan :
   a) both columns are of integer Type.
   b) 1 column is having index and another is non key column.
   c) condition: The column having SAFE FUNCTION against Index column in WHERE condition.
*/
CREATE or replace function fun_pis (n int) returns int parallel safe  as $$ begin return 1000; end; $$ language 'plpgsql';
explain  analyze verbose select aid from pgbench_accounts where aid > fun_pis(9) and aid < 9000 ;
-- TestCase: 4
--Description 
/*
Explain plan catching Parallel index only scan when  in WHERE condition
   a) 3 columns, 1 column is having PRIMARY KEY on "int" Datatype and another non key columns having "int" and "char" datatype.
   b) condition: WHERE clause having 3 conditions, index column is selecting more records as compaired to other column conditions.
-- Need to disable SEQUENTIAL  to reproduce explain plan catches "parallel index Only scan"
*/
\c test
CREATE TABLE tt2(c1 serial primary key, c2 int, c3 char(10));
INSERT INTO tt2(c2, c3) VALUES (generate_series(1,30), 'abc');
INSERT INTO tt2(c2, c3) VALUES (generate_series(31,100), 'pqrs');
analyze tt2;
set enable_seqscan =0;
explain analyze select count(c1) from tt2 where c1 < 00;

-- TestCase: 5
--Description
/*
Explain plan catching Parallel index only scan when in WHERE condition
   a) 3 columns, 2 column is having composite index on "int" and "character" Datatype and another non key columns having "int" datatype.
   b) condition: WHERE clause having 1 multi-column condition selecting few records.
  -- Need to disable BITMAPSCAN, SEQUENTIALSCAN to reproduce explain plan catch "parallel index only scan"
*/

set enable_seqscan =0;
explain analyze verbose select count(*) from tt2 where (c1) NOT  IN((100));
-- TestCase: 6
--Description 
/*
Explain plan catching Parallel index only scan when in WHERE condition
   a) 2 columns, 1 non-key column having "text" datatype and another column having "array of integer[]" Datatype having index.
   b) condition: WHERE clause having 2 conditions, the array index column is selecting more records as compaired to other non key column condition.
*/
CREATE TABLE ary_tab (c1 text, c2 integer[]);
INSERT INTO ary_tab VALUES ('one', '{1,2,3}');
INSERT INTO ary_tab VALUES ('two', '{4,5,6}');
INSERT INTO ary_tab VALUES ('three', '{2,4,6}');
INSERT INTO ary_tab  (select 'four', '{7,8,9,10}' from generate_series(1,50));
INSERT INTO ary_tab  (select 'five', '{7,8,9,10}' from generate_series(1,100));
CREATE INDEX ary_idx on ary_tab (c2);
analyze;
explain analyze  verbose select count(*) from ary_tab where ARRAY[7,8,9,10]=c2 ;

-- TestCase: 7
--Description 
/*
Explain plan catching Parallel index only scan when in WHERE condition
   a) 4 columns, 1 non-key column having "TEXT" datatype and others are "INTEGER", "FLOAT", "VARCHAR" column having "COMPOSIT INDEX", and the same "INTEGER" column have "INDEX".
   b) condition: WHERE clause having 1 conditions, the index column is selecting more records.
  -- Need to disable SEQUENTIALSCAN /BITMAP  to reproduce explain plan catch "parallel index only scan"
*/
\c test
CREATE TABLE tst_pis(c1 int, c2 text, c3 float, c4 varchar(10));
INSERT INTO tst_pis (select x, 'c2_'||x, x/3,'c4_'||x from generate_series(1,100) x);
CREATE INDEX 

Re: [HACKERS] Parallel Index-only scan

[Rafia Sabih]

On Fri, Jan 13, 2017 at 2:19 PM, Rafia Sabih
 wrote:
> On Thu, Jan 12, 2017 at 5:39 PM, Rahila Syed  wrote:
>> Hello,
>>
>> On applying the patch on latest master branch and running regression tests
>> following failure occurs.
>> I applied it on latest parallel index scan patches as given in the link
>> above.
>>
>> ***
>> /home/rahila/postgres/postgres/src/test/regress/expected/select_parallel.out
>> 2017-01-03 14:06:29.122022780 +0530
>> ---
>> /home/rahila/postgres/postgres/src/test/regress/results/select_parallel.out
>> 2017-01-12 14:35:56.652712622 +0530
>> ***
>> *** 92,103 
>>   explain (costs off)
>> select  sum(parallel_restricted(unique1)) from tenk1
>> group by(parallel_restricted(unique1));
>> !  QUERY PLAN
>> ! 
>>HashAggregate
>>  Group Key: parallel_restricted(unique1)
>> !->  Index Only Scan using tenk1_unique1 on tenk1
>> ! (3 rows)
>>
>>   set force_parallel_mode=1;
>>   explain (costs off)
>> --- 92,105 
>>   explain (costs off)
>> select  sum(parallel_restricted(unique1)) from tenk1
>> group by(parallel_restricted(unique1));
>> ! QUERY PLAN
>> ! ---
>>HashAggregate
>>  Group Key: parallel_restricted(unique1)
>> !->  Gather
>> !  Workers Planned: 4
>> !  ->  Parallel Index Only Scan using tenk1_unique1 on tenk1
>> ! (5 rows)
>>
>> IIUC, parallel operation being performed here is fine as parallel restricted
>> function occurs above Gather node
>> and just the expected output needs to be changed.
>>
>
> True, fixed it, please find the attached file for the latest version.
>

Please find the attached file rebased patch of parallel index-only
scan on the latest Parallel index-scan patch [1].

[1] 
https://www.postgresql.org/message-id/CAA4eK1L-gb0Fum3mQN4c5PWJXNE7xs7pzwMDWsrDYLucKqvJ2A%40mail.gmail.com
-- 
Regards,
Rafia Sabih
EnterpriseDB: http://www.enterprisedb.com/


parallel_index_only_v5.patch
Description: Binary data

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] Parallel Index-only scan

[Rafia Sabih]

On Thu, Jan 12, 2017 at 5:39 PM, Rahila Syed  wrote:
> Hello,
>
> On applying the patch on latest master branch and running regression tests
> following failure occurs.
> I applied it on latest parallel index scan patches as given in the link
> above.
>
> ***
> /home/rahila/postgres/postgres/src/test/regress/expected/select_parallel.out
> 2017-01-03 14:06:29.122022780 +0530
> ---
> /home/rahila/postgres/postgres/src/test/regress/results/select_parallel.out
> 2017-01-12 14:35:56.652712622 +0530
> ***
> *** 92,103 
>   explain (costs off)
> select  sum(parallel_restricted(unique1)) from tenk1
> group by(parallel_restricted(unique1));
> !  QUERY PLAN
> ! 
>HashAggregate
>  Group Key: parallel_restricted(unique1)
> !->  Index Only Scan using tenk1_unique1 on tenk1
> ! (3 rows)
>
>   set force_parallel_mode=1;
>   explain (costs off)
> --- 92,105 
>   explain (costs off)
> select  sum(parallel_restricted(unique1)) from tenk1
> group by(parallel_restricted(unique1));
> ! QUERY PLAN
> ! ---
>HashAggregate
>  Group Key: parallel_restricted(unique1)
> !->  Gather
> !  Workers Planned: 4
> !  ->  Parallel Index Only Scan using tenk1_unique1 on tenk1
> ! (5 rows)
>
> IIUC, parallel operation being performed here is fine as parallel restricted
> function occurs above Gather node
> and just the expected output needs to be changed.
>

True, fixed it, please find the attached file for the latest version.

> Thank you,
> Rahila Syed
>
Thanks a lot for the review Rahila.
-- 
Regards,
Rafia Sabih
EnterpriseDB: http://www.enterprisedb.com/


parallel_index_only_v4.patch
Description: Binary data

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] Parallel Index-only scan

[Rahila Syed]

Hello,

On applying the patch on latest master branch and running regression tests
following failure occurs.
I applied it on latest parallel index scan patches as given in the link
above.

***
/home/rahila/postgres/postgres/src/test/regress/expected/select_parallel.out
2017-01-03 14:06:29.122022780 +0530
---
/home/rahila/postgres/postgres/src/test/regress/results/select_parallel.out
2017-01-12 14:35:56.652712622 +0530
***
*** 92,103 
  explain (costs off)
select  sum(parallel_restricted(unique1)) from tenk1
group by(parallel_restricted(unique1));
!  QUERY PLAN
! 
   HashAggregate
 Group Key: parallel_restricted(unique1)
!->  Index Only Scan using tenk1_unique1 on tenk1
! (3 rows)

  set force_parallel_mode=1;
  explain (costs off)
--- 92,105 
  explain (costs off)
select  sum(parallel_restricted(unique1)) from tenk1
group by(parallel_restricted(unique1));
! QUERY PLAN
! ---
   HashAggregate
 Group Key: parallel_restricted(unique1)
!->  Gather
!  Workers Planned: 4
!  ->  Parallel Index Only Scan using tenk1_unique1 on tenk1
! (5 rows)

IIUC, parallel operation being performed here is fine as parallel
restricted function occurs above Gather node
and just the expected output needs to be changed.

Thank you,
Rahila Syed


regression.diffs
Description: Binary data

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] Parallel Index-only scan

[Amit Kapila]

On Wed, Dec 28, 2016 at 12:18 PM, Rafia Sabih
 wrote:
> Rebased patch of parallel-index only scan based on the latest version of
> parallel index scan [1] is attached.
>
> [1]
> https://www.postgresql.org/message-id/CAA4eK1LiNi7_Z1%2BPCV4y06o_v%3DZdZ1UThE%2BW9JhthX4B8uifnA%40mail.gmail.com
>

The link for the latest version is wrong, the correct link is:
https://www.postgresql.org/message-id/CAA4eK1KthrAvNjmB2cWuUHz%2Bp3ZTTtbD7o2KUw49PC8HK4r1Wg%40mail.gmail.com

-- 
With Regards,
Amit Kapila.
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

Re: [HACKERS] Parallel Index-only scan

[Rafia Sabih]

Rebased patch of parallel-index only scan based on the latest version of
parallel index scan [1] is attached.

[1] https://www.postgresql.org/message-id/CAA4eK1LiNi7_Z1%
2BPCV4y06o_v%3DZdZ1UThE%2BW9JhthX4B8uifnA%40mail.gmail.com

On Sat, Dec 24, 2016 at 7:55 PM, Rafia Sabih 
wrote:

> Extremely sorry for the inconvenience caused, please find the attached
> file for the latest version of the patch.
>
> On Sat, Dec 24, 2016 at 1:41 AM, Robert Haas 
> wrote:
>
>> On Fri, Dec 23, 2016 at 3:03 PM, Tom Lane  wrote:
>> > Or in words of one syllable: please do not use nabble to post to the
>> > community mailing lists.
>>
>> Many of those words have two syllables, and one has four.
>>
>> Anyhow, I think three castigating emails from committers in a span of
>> 62 minutes is probably enough for the OP to get the point, unless
>> someone else REALLY feels the need to pile on.
>>
>> --
>> Robert Haas
>> EnterpriseDB: http://www.enterprisedb.com
>> The Enterprise PostgreSQL Company
>>
>
>
>
> --
> Regards,
> Rafia Sabih
> EnterpriseDB: http://www.enterprisedb.com/
>



-- 
Regards,
Rafia Sabih
EnterpriseDB: http://www.enterprisedb.com/


parallel_index_only_v3.patch
Description: Binary data

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] Parallel Index-only scan

[Rafia Sabih]

Extremely sorry for the inconvenience caused, please find the attached file
for the latest version of the patch.

On Sat, Dec 24, 2016 at 1:41 AM, Robert Haas  wrote:

> On Fri, Dec 23, 2016 at 3:03 PM, Tom Lane  wrote:
> > Or in words of one syllable: please do not use nabble to post to the
> > community mailing lists.
>
> Many of those words have two syllables, and one has four.
>
> Anyhow, I think three castigating emails from committers in a span of
> 62 minutes is probably enough for the OP to get the point, unless
> someone else REALLY feels the need to pile on.
>
> --
> Robert Haas
> EnterpriseDB: http://www.enterprisedb.com
> The Enterprise PostgreSQL Company
>



-- 
Regards,
Rafia Sabih
EnterpriseDB: http://www.enterprisedb.com/


parallel_index_only_v2.patch
Description: Binary data

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] Parallel Index-only scan

[Robert Haas]

On Fri, Dec 23, 2016 at 3:03 PM, Tom Lane  wrote:
> Or in words of one syllable: please do not use nabble to post to the
> community mailing lists.

Many of those words have two syllables, and one has four.

Anyhow, I think three castigating emails from committers in a span of
62 minutes is probably enough for the OP to get the point, unless
someone else REALLY feels the need to pile on.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] Parallel Index-only scan

[Tom Lane]

Robert Haas  writes:
> On Fri, Dec 23, 2016 at 12:04 AM, rafia.sabih
>  wrote:
>> Please find the attached file for the latest version of patch.
>> parallel_index_only_v2.patch
>> 

> We want to have the patch actually attached, not just stored on nabble.

Or in words of one syllable: please do not use nabble to post to the
community mailing lists.  Their habit of stripping attachments is not
the only evil thing about them (although it's the only one I remember
details of ATM).

regards, tom lane


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] Parallel Index-only scan

[Alvaro Herrera]

Robert Haas wrote:
> On Fri, Dec 23, 2016 at 12:04 AM, rafia.sabih
>  wrote:
> > Please find the attached file for the latest version of patch.
> >
> > parallel_index_only_v2.patch
> > 
> 
> We want to have the patch actually attached, not just stored on nabble.

I think the problem is that Nabble removes the attachment before
resending the email to the list.  The OP should be posting directly
instead of via Nabble.

-- 
Álvaro Herrerahttps://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] Parallel Index-only scan

[Robert Haas]

On Fri, Dec 23, 2016 at 12:04 AM, rafia.sabih
 wrote:
> Please find the attached file for the latest version of patch.
>
> parallel_index_only_v2.patch
> 

We want to have the patch actually attached, not just stored on nabble.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] Parallel Index-only scan

[rafia.sabih]

Please find the attached file for the latest version of patch.

parallel_index_only_v2.patch
  



--
View this message in context: 
http://postgresql.nabble.com/Parallel-Index-only-scan-tp5934352p5936010.html
Sent from the PostgreSQL - hackers mailing list archive at Nabble.com.


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

[HACKERS] Parallel Index-only scan

[Rafia Sabih]

Hello all,
This is to propose a patch for enabling parallel index-only scans. With the
patch of parallel index scan around [1], adding the mechanism for
parallelising index-only scans makes sense. Without this mechanism for the
queries preferring index-only scans, it is likely that at higher scale
parallel index or parallel seq scan serve as cheaper alternative to
index-only scans and then query performance might suffer because of the
added processing of index or seq scans.

Performance
-
Consider the performance of a simple query on TPC-H schema,

explain analyse select count(*) from lineitem where l_shipdate < date
'1995-01-03';

Without parallel index-only scan, parallel seq scan got picked and it took
around 23 seconds for the query to execute,

> Finalize Aggregate  (cost=651586.63..651586.64 rows=1 width=8) (actual
> time=22853.872..22853.873 rows=1 loops=1)
>->  Gather  (cost=651586.21..651586.62 rows=4 width=8) (actual
> time=22853.684..22853.864 rows=5 loops=1)
>  Workers Planned: 4
>  Workers Launched: 4
>  ->  Partial Aggregate  (cost=650586.21..650586.22 rows=1 width=8)
> (actual time=22850.489..22850.489 rows=1 loops=5)
>->  Parallel Seq Scan on lineitem  (cost=0.00..618021.73
> rows=13025795 width=0) (actual time=0.035..20553.495 rows=10342437 loops=5)
>  Filter: (l_shipdate < '1995-01-03'::date)
>  Rows Removed by Filter: 13656485
>  Planning time: 0.225 ms
>  Execution time: 22855.196 ms
>
However, with parallel index-only scan, it took only 8.5 seconds,

Finalize Aggregate  (cost=568883.69..568883.70 rows=1 width=8) (actual
> time=8548.993..8548.993 rows=1 loops=1)
>->  Gather  (cost=568883.27..568883.68 rows=4 width=8) (actual
> time=8548.789..8548.976 rows=5 loops=1)
>  Workers Planned: 4
>  Workers Launched: 4
>  ->  Partial Aggregate  (cost=567883.27..567883.28 rows=1 width=8)
> (actual time=8541.929..8541.929 rows=1 loops=5)
>->  Parallel Index Only Scan using idx_l_shipdate on
> lineitem  (cost=0.57..535318.78 rows=13025795 width=0) (actual
> time=0.113..5866.729 rows=10342437 loops=5)
>  Index Cond: (l_shipdate < '1995-01-03'::date)
>  Heap Fetches: 0
>  Planning time: 0.266 ms
>  Execution time: 8569.735 ms


The effect of parallel index-only scan can be seen more in some more
complex queries where parallelism is enabled till top of the tree, e.g,
following query takes 118 ms on head,

explain analyse select sum(l_extendedprice * l_discount) as revenue,
avg(o_orderkey) from orders, lineitem where o_orderkey < 1 and
o_orderkey = l_orderkey group by l_shipmode order by revenue

 Sort  (cost=24396.44..24396.45 rows=7 width=75) (actual
> time=118.823..118.825 rows=7 loops=1)
>Sort Key: (sum((lineitem.l_extendedprice * lineitem.l_discount)))
>Sort Method: quicksort  Memory: 25kB
>->  HashAggregate  (cost=24396.23..24396.34 rows=7 width=75) (actual
> time=118.749..118.786 rows=7 loops=1)
>  Group Key: lineitem.l_shipmode
>  ->  Nested Loop  (cost=1.13..24293.11 rows=10312 width=27)
> (actual time=0.096..73.198 rows=9965 loops=1)
>->  Index Only Scan using orders_pkey on orders
>  (cost=0.56..46.48 rows=2578 width=4) (actual time=0.072..1.663 rows=2503
> loops=1)
>  Index Cond: (o_orderkey < 1)
>  Heap Fetches: 0
>->  Index Scan using idx_lineitem_orderkey on lineitem
>  (cost=0.57..6.45 rows=296 width=31) (actual time=0.018..0.023 rows=4
> loops=2503)
>  Index Cond: (l_orderkey = orders.o_orderkey)
>  Planning time: 1.062 ms
>  Execution time: 118.977 ms


With parallel index-only scan, the performance improves to 40 ms,

Sort  (cost=7191.33..7191.35 rows=7 width=75) (actual time=40.475..40.476
> rows=7 loops=1)
>Sort Key: (sum((lineitem.l_extendedprice * lineitem.l_discount)))
>Sort Method: quicksort  Memory: 25kB
>->  Finalize GroupAggregate  (cost=7190.78..7191.23 rows=7 width=75)
> (actual time=40.168..40.451 rows=7 loops=1)
>  Group Key: lineitem.l_shipmode
>  ->  Sort  (cost=7190.78..7190.85 rows=28 width=75) (actual
> time=40.105..40.127 rows=35 loops=1)
>Sort Key: lineitem.l_shipmode
>Sort Method: quicksort  Memory: 29kB
>->  Gather  (cost=7187.22..7190.11 rows=28 width=75)
> (actual time=39.344..39.983 rows=35 loops=1)
>  Workers Planned: 4
>  Workers Launched: 4
>  ->  Partial HashAggregate  (cost=6187.22..6187.31
> rows=7 width=75) (actual time=25.981..26.011 rows=7 loops=5)
>Group Key: lineitem.l_shipmode
>->  Nested Loop  (cost=1.13..6084.10 rows=10312
> width=27) (actual time=0.139..16.352 rows=1993 loops=5)
>  ->  Parallel Index Only Scan using
>