[postgresql 10 beta3] unrecognized node type: 90

Started by Adam, Etienne (Nokia-TECH/Issy Les Moulineaux)over 8 years ago44 messageshackersbugs
Jump to latest
hackersbugs

Hi,

While testing the beta3 of PostgreSQL 10 (Ubuntu/10~beta3-1.pgdg16.04+1) on
a database restored from production data, executing the following query
(field and table names have been changed):

SELECT id_c
FROM c
LEFT OUTER JOIN (
SELECT h_c AS h_c_sub, count(*) AS num
FROM r, c
WHERE id_a_r = 1308 AND id_c = id_c_r
GROUP BY h_c
) AS sent ON h_c = h_c_sub
WHERE ((date_trunc('days', age('2017-08-10', to_timestamp(d1_c))) > '11
days') AND ((h->'x') IS NULL) AND ((h->'x') IS NULL) AND ((d2_c IS NOT NULL
AND date_trunc('days', age('2017-08-10', to_timestamp(d2_c))) <= '10
days')))
AND NOT bool1 AND string1 IS NOT NULL AND (bool2 IS NULL OR bool2 = true)
AND coalesce(num, 0) < 1 AND coalesce(bool3, TRUE) IS TRUE;

We encountered the following error (the query executes fine on a 9.2
server):

ERROR: XX000: unrecognized node type: 90
LOCATION: ExecReScan, execAmi.c:284

If we modify it just a little bit (removing a WHERE clause), it works:

SELECT id_c
FROM c
LEFT OUTER JOIN (
SELECT h_c AS h_c_sub, count(*) AS num
FROM r, c
WHERE id_a_r = 1308 AND id_c = id_c_r
GROUP BY h_c
) AS sent ON h_c = h_c_sub
WHERE ((date_trunc('days', age('2017-08-10', to_timestamp(d1_c))) > '11
days') AND ((h->'x') IS NULL) AND ((h->'x') IS NULL))
AND NOT bool1 AND string1 IS NOT NULL AND (bool2 IS NULL OR bool2 = true)
AND coalesce(num, 0) < 1 AND coalesce(bool3, TRUE) IS TRUE;

We know it is a beta version but we wanted to report it in case it was not
a known bug. Please let us know if you need anything more,

Best,
Adam Etienne

#2Tom Lane
tgl@sss.pgh.pa.us
In reply to: Adam, Etienne (Nokia-TECH/Issy Les Moulineaux) (#1)
hackersbugs
Re: [postgresql 10 beta3] unrecognized node type: 90

"Adam, Etienne (Nokia-TECH/Issy Les Moulineaux)" <etienne.adam@nokia.com> writes:

ERROR: XX000: unrecognized node type: 90
LOCATION: ExecReScan, execAmi.c:284

(gdb) p (NodeTag) 90
$1 = T_GatherMergeState

So, apparently somebody wrote ExecReScanGatherMerge, but never bothered
to plug it into ExecReScan. From which we may draw depressing conclusions
about how much it's been tested.

Thanks for the report!

regards, tom lane

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

#3Tom Lane
tgl@sss.pgh.pa.us
In reply to: Tom Lane (#2)
hackersbugs
Re: [postgresql 10 beta3] unrecognized node type: 90

I wrote:

So, apparently somebody wrote ExecReScanGatherMerge, but never bothered
to plug it into ExecReScan. From which we may draw depressing conclusions
about how much it's been tested.

While I'm bitching ... the code coverage report at

https://coverage.postgresql.org/src/backend/executor/nodeGatherMerge.c.gcov.html

also leaves one with less than a warm feeling about the extent of test
coverage on this file. heap_compare_slots isn't invoked even once?

regards, tom lane

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

#4Andres Freund
andres@anarazel.de
In reply to: Tom Lane (#3)
hackersbugs
Re: [postgresql 10 beta3] unrecognized node type: 90

On 2017-08-11 16:20:51 -0400, Tom Lane wrote:

I wrote:

So, apparently somebody wrote ExecReScanGatherMerge, but never bothered
to plug it into ExecReScan. From which we may draw depressing conclusions
about how much it's been tested.

While I'm bitching ... the code coverage report at

https://coverage.postgresql.org/src/backend/executor/nodeGatherMerge.c.gcov.html

also leaves one with less than a warm feeling about the extent of test
coverage on this file. heap_compare_slots isn't invoked even once?

I complained about this before at
http://archives.postgresql.org/message-id/20170401022605.4wag26gtyzhny7ue%40alap3.anarazel.de
but I just noticed that Rushabh appears to have sent a patch adding
coverage. Missed that somehow, will apply.

- Andres

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

#5Andres Freund
andres@anarazel.de
In reply to: Andres Freund (#4)
hackersbugs
Re: [postgresql 10 beta3] unrecognized node type: 90

On 2017-08-11 13:26:29 -0700, Andres Freund wrote:

On 2017-08-11 16:20:51 -0400, Tom Lane wrote:

I wrote:

So, apparently somebody wrote ExecReScanGatherMerge, but never bothered
to plug it into ExecReScan. From which we may draw depressing conclusions
about how much it's been tested.

While I'm bitching ... the code coverage report at

https://coverage.postgresql.org/src/backend/executor/nodeGatherMerge.c.gcov.html

also leaves one with less than a warm feeling about the extent of test
coverage on this file. heap_compare_slots isn't invoked even once?

I complained about this before at
http://archives.postgresql.org/message-id/20170401022605.4wag26gtyzhny7ue%40alap3.anarazel.de
but I just noticed that Rushabh appears to have sent a patch adding
coverage. Missed that somehow, will apply.

And pushed (to both 10 and master).

- Andres

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

#6Noah Misch
noah@leadboat.com
In reply to: Tom Lane (#2)
hackersbugs
Re: [postgresql 10 beta3] unrecognized node type: 90

On Fri, Aug 11, 2017 at 11:59:14AM -0400, Tom Lane wrote:

"Adam, Etienne (Nokia-TECH/Issy Les Moulineaux)" <etienne.adam@nokia.com> writes:

ERROR: XX000: unrecognized node type: 90
LOCATION: ExecReScan, execAmi.c:284

(gdb) p (NodeTag) 90
$1 = T_GatherMergeState

So, apparently somebody wrote ExecReScanGatherMerge, but never bothered
to plug it into ExecReScan. From which we may draw depressing conclusions
about how much it's been tested.

[Action required within three days. This is a generic notification.]

The above-described topic is currently a PostgreSQL 10 open item. Robert,
since you committed the patch believed to have created it, you own this open
item. If some other commit is more relevant or if this does not belong as a
v10 open item, please let us know. Otherwise, please observe the policy on
open item ownership[1]/messages/by-id/20170404140717.GA2675809@tornado.leadboat.com and send a status update within three calendar days of
this message. Include a date for your subsequent status update. Testers may
discover new open items at any time, and I want to plan to get them all fixed
well in advance of shipping v10. Consequently, I will appreciate your efforts
toward speedy resolution. Thanks.

[1]: /messages/by-id/20170404140717.GA2675809@tornado.leadboat.com

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

#7Amit Kapila
amit.kapila16@gmail.com
In reply to: Noah Misch (#6)
hackersbugs
Re: [postgresql 10 beta3] unrecognized node type: 90

On Tue, Aug 15, 2017 at 8:22 AM, Noah Misch <noah@leadboat.com> wrote:

On Fri, Aug 11, 2017 at 11:59:14AM -0400, Tom Lane wrote:

"Adam, Etienne (Nokia-TECH/Issy Les Moulineaux)" <etienne.adam@nokia.com> writes:

ERROR: XX000: unrecognized node type: 90
LOCATION: ExecReScan, execAmi.c:284

(gdb) p (NodeTag) 90
$1 = T_GatherMergeState

So, apparently somebody wrote ExecReScanGatherMerge, but never bothered
to plug it into ExecReScan.

Attached patch fixes the issue for me. I have locally verified that
the gather merge gets executed in rescan path. I haven't added a test
case for the same as having gather or gather merge on the inner side
of join can be time-consuming. However, if you or others feel that it
is important to have a test to cover this code path, then I can try to
produce one.

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

Attachments:

execrescan_gathermerge_v1.patchapplication/octet-stream; name=execrescan_gathermerge_v1.patchDownload+5-0
#8Robert Haas
robertmhaas@gmail.com
In reply to: Amit Kapila (#7)
hackersbugs
Re: [BUGS] [postgresql 10 beta3] unrecognized node type: 90

On Tue, Aug 15, 2017 at 7:31 AM, Amit Kapila <amit.kapila16@gmail.com> wrote:

On Tue, Aug 15, 2017 at 8:22 AM, Noah Misch <noah@leadboat.com> wrote:

On Fri, Aug 11, 2017 at 11:59:14AM -0400, Tom Lane wrote:

"Adam, Etienne (Nokia-TECH/Issy Les Moulineaux)" <etienne.adam@nokia.com> writes:

ERROR: XX000: unrecognized node type: 90
LOCATION: ExecReScan, execAmi.c:284

(gdb) p (NodeTag) 90
$1 = T_GatherMergeState

So, apparently somebody wrote ExecReScanGatherMerge, but never bothered
to plug it into ExecReScan.

Attached patch fixes the issue for me. I have locally verified that
the gather merge gets executed in rescan path. I haven't added a test
case for the same as having gather or gather merge on the inner side
of join can be time-consuming. However, if you or others feel that it
is important to have a test to cover this code path, then I can try to
produce one.

Committed.

I believe that between this commit and the test-coverage commit from
Andres, this open item is reasonably well addressed. If someone
thinks more needs to be done, please specify. Thanks.

--
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

#9Tom Lane
tgl@sss.pgh.pa.us
In reply to: Robert Haas (#8)
hackersbugs
Re: [HACKERS] [postgresql 10 beta3] unrecognized node type: 90

Robert Haas <robertmhaas@gmail.com> writes:

On Tue, Aug 15, 2017 at 7:31 AM, Amit Kapila <amit.kapila16@gmail.com> wrote:

Attached patch fixes the issue for me. I have locally verified that
the gather merge gets executed in rescan path. I haven't added a test
case for the same as having gather or gather merge on the inner side
of join can be time-consuming. However, if you or others feel that it
is important to have a test to cover this code path, then I can try to
produce one.

Committed.

I believe that between this commit and the test-coverage commit from
Andres, this open item is reasonably well addressed. If someone
thinks more needs to be done, please specify. Thanks.

How big a deal do we think test coverage is? It looks like
ExecReScanGatherMerge is identical logic to ExecReScanGather,
which *is* covered according to coverage.postgresql.org, but
it wouldn't be too surprising if they diverge in future.

I should think it wouldn't be that expensive to create a test
case, if you already have test cases that invoke GatherMerge.
Adding a right join against a VALUES clause with a small number of
entries, and a non-mergeable/hashable join clause, ought to do it.

regards, tom lane

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

#10Robert Haas
robertmhaas@gmail.com
In reply to: Tom Lane (#9)
hackersbugs
Re: [BUGS] [postgresql 10 beta3] unrecognized node type: 90

On Tue, Aug 15, 2017 at 9:46 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote:

How big a deal do we think test coverage is? It looks like
ExecReScanGatherMerge is identical logic to ExecReScanGather,
which *is* covered according to coverage.postgresql.org, but
it wouldn't be too surprising if they diverge in future.

I should think it wouldn't be that expensive to create a test
case, if you already have test cases that invoke GatherMerge.
Adding a right join against a VALUES clause with a small number of
entries, and a non-mergeable/hashable join clause, ought to do it.

I chatted with Amit about this -- he's planning to look into it. I
assume we'll hear from him tomorrow about this, but for official
status update purposes I'll set a next-update date of one week from
today (August 23rd).

--
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

#11Amit Kapila
amit.kapila16@gmail.com
In reply to: Tom Lane (#9)
hackersbugs
Re: [BUGS] [postgresql 10 beta3] unrecognized node type: 90

On Tue, Aug 15, 2017 at 7:16 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:

Robert Haas <robertmhaas@gmail.com> writes:

On Tue, Aug 15, 2017 at 7:31 AM, Amit Kapila <amit.kapila16@gmail.com> wrote:

Attached patch fixes the issue for me. I have locally verified that
the gather merge gets executed in rescan path. I haven't added a test
case for the same as having gather or gather merge on the inner side
of join can be time-consuming. However, if you or others feel that it
is important to have a test to cover this code path, then I can try to
produce one.

Committed.

I believe that between this commit and the test-coverage commit from
Andres, this open item is reasonably well addressed. If someone
thinks more needs to be done, please specify. Thanks.

How big a deal do we think test coverage is? It looks like
ExecReScanGatherMerge is identical logic to ExecReScanGather,
which *is* covered according to coverage.postgresql.org, but
it wouldn't be too surprising if they diverge in future.

I should think it wouldn't be that expensive to create a test
case, if you already have test cases that invoke GatherMerge.
Adding a right join against a VALUES clause with a small number of
entries, and a non-mergeable/hashable join clause, ought to do it.

I have done some experiments based on this idea to generate a test,
but I think it is not as straightforward as it appears. Below are
some of my experiments:

Query that uses GatherMerge in regression tests
---------------------------------------------------------------------

regression=# explain (costs off) select string4, count((unique2))
from tenk1 group by string4 order by string4;
QUERY PLAN
----------------------------------------------------
Finalize GroupAggregate
Group Key: string4
-> Gather Merge
Workers Planned: 2
-> Partial GroupAggregate
Group Key: string4
-> Sort
Sort Key: string4
-> Parallel Seq Scan on tenk1
(9 rows)

Modified Query
----------------------
regression=# explain (costs off) select tenk1.hundred,
count((unique2)) from tenk1 right join (values (1,100), (2,200)) as t
(two, hundred) on t.two

1 and tenk1.hundred > 1 group by tenk1.hundred order by tenk1.hundred;

QUERY PLAN
--------------------------------------------------------------------------
Sort
Sort Key: tenk1.hundred
-> HashAggregate
Group Key: tenk1.hundred
-> Nested Loop Left Join
Join Filter: ("*VALUES*".column1 > 1)
-> Values Scan on "*VALUES*"
-> Gather
Workers Planned: 4
-> Parallel Index Scan using tenk1_hundred on tenk1
Index Cond: (hundred > 1)
(11 rows)

The cost of GatherMerge is always higher than Gather in this case
which is quite obvious as GatherMerge has to perform some additional
task. I am not aware of a way such that Grouping and Sorting can be
pushed below parallel node (Gather/GatherMerge) in this case, if there
is any such possibility, then it might prefer GatherMerge.

Another way to make it parallel is, add a new guc enable_gather
similar to enable_gathermerge and then set that to off, it will prefer
GatherMerge in that case. I think it is anyway good to have such a
guc. I will go and do it this way unless you have a better idea.

Note - enable_gathermerge is not present in postgresql.conf. I think
we should add it in the postgresql.conf.sample file.

--
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

#12Thomas Munro
thomas.munro@gmail.com
In reply to: Amit Kapila (#11)
hackersbugs
Re: [HACKERS] [postgresql 10 beta3] unrecognized node type: 90

On Thu, Aug 17, 2017 at 4:37 PM, Amit Kapila <amit.kapila16@gmail.com> wrote:

Note - enable_gathermerge is not present in postgresql.conf. I think
we should add it in the postgresql.conf.sample file.

+1

See also /messages/by-id/CAEepm=0B7yM9MZSviq1d-hnt4KoaRVeJvSfAyVfykNV-pVDqug@mail.gmail.com
.

--
Thomas Munro
http://www.enterprisedb.com

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

#13Amit Kapila
amit.kapila16@gmail.com
In reply to: Amit Kapila (#11)
hackersbugs
Re: [HACKERS] [postgresql 10 beta3] unrecognized node type: 90

On Thu, Aug 17, 2017 at 10:07 AM, Amit Kapila <amit.kapila16@gmail.com> wrote:

On Tue, Aug 15, 2017 at 7:16 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:

I believe that between this commit and the test-coverage commit from
Andres, this open item is reasonably well addressed. If someone
thinks more needs to be done, please specify. Thanks.

How big a deal do we think test coverage is? It looks like
ExecReScanGatherMerge is identical logic to ExecReScanGather,
which *is* covered according to coverage.postgresql.org, but
it wouldn't be too surprising if they diverge in future.

I should think it wouldn't be that expensive to create a test
case, if you already have test cases that invoke GatherMerge.
Adding a right join against a VALUES clause with a small number of
entries, and a non-mergeable/hashable join clause, ought to do it.

Another way to make it parallel is, add a new guc enable_gather
similar to enable_gathermerge and then set that to off, it will prefer
GatherMerge in that case. I think it is anyway good to have such a
guc. I will go and do it this way unless you have a better idea.

Going by above, I have created two separate patches. First to
introduce a new guc enable_gather and second patch to test the rescan
behavior of gather merge. I have found a problem in the rescan path
of gather merge which is that it is not initializing the gather merge
state which is required to initialize the heap for processing of
tuples. I think this should have been caught earlier, but probably I
didn't notice it because in the previous tests left side would not
have passed enough rows to hit this case. I have fixed it in the
attached patch (execrescan_gathermerge_v2).

Note - enable_gathermerge is not present in postgresql.conf. I think
we should add it in the postgresql.conf.sample file.

Thomas has already posted a patch to handle this problem.

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

Attachments:

guc_enable_gather_v1.patchapplication/octet-stream; name=guc_enable_gather_v1.patchDownload+18-2
execrescan_gathermerge_v2.patchapplication/octet-stream; name=execrescan_gathermerge_v2.patchDownload+73-0
#14Tom Lane
tgl@sss.pgh.pa.us
In reply to: Amit Kapila (#11)
hackersbugs
Re: [HACKERS] [postgresql 10 beta3] unrecognized node type: 90

Amit Kapila <amit.kapila16@gmail.com> writes:

On Tue, Aug 15, 2017 at 7:16 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:

I should think it wouldn't be that expensive to create a test
case, if you already have test cases that invoke GatherMerge.
Adding a right join against a VALUES clause with a small number of
entries, and a non-mergeable/hashable join clause, ought to do it.

I have done some experiments based on this idea to generate a test,
but I think it is not as straightforward as it appears.

I did this (the first 4 SETs duplicate what's already used in
select_parallel.sql):

regression=# set parallel_setup_cost=0;
SET
regression=# set parallel_tuple_cost=0;
SET
regression=# set min_parallel_table_scan_size=0;
SET
regression=# set max_parallel_workers_per_gather=4;
SET
regression=# set enable_hashagg TO 0;
SET
regression=# set enable_material TO 0;
SET
regression=# explain select * from (select string4, count((unique2))
from tenk1 group by string4 order by string4) ss right join
(values(1),(2)) v(x) on true;
QUERY PLAN
--------------------------------------------------------------------------------------------------
Nested Loop Left Join (cost=524.15..1086.77 rows=8 width=76)
-> Values Scan on "*VALUES*" (cost=0.00..0.03 rows=2 width=4)
-> Finalize GroupAggregate (cost=524.15..543.29 rows=4 width=72)
Group Key: tenk1.string4
-> Gather Merge (cost=524.15..543.17 rows=16 width=72)
Workers Planned: 4
-> Partial GroupAggregate (cost=524.10..542.89 rows=4 width=72)
Group Key: tenk1.string4
-> Sort (cost=524.10..530.35 rows=2500 width=68)
Sort Key: tenk1.string4
-> Parallel Seq Scan on tenk1 (cost=0.00..383.00 rows=2500 width=68)
(11 rows)

regression=# select * from (select string4, count((unique2))
from tenk1 group by string4 order by string4) ss right join
(values(1),(2)) v(x) on true;
server closed the connection unexpectedly

So, not only is it not that hard to reach ExecReScanGatherMerge,
but there is indeed a bug to fix there somewhere. The stack
trace indicates that the failure occurs in a later execution
of ExecGatherMerge:

Program terminated with signal 11, Segmentation fault.
#0 0x000000000064b4e4 in swap_nodes (heap=0x15a9440) at binaryheap.c:223
223 heap->bh_nodes[a] = heap->bh_nodes[b];
(gdb) bt
#0 0x000000000064b4e4 in swap_nodes (heap=0x15a9440) at binaryheap.c:223
#1 binaryheap_remove_first (heap=0x15a9440) at binaryheap.c:189
#2 0x0000000000634196 in gather_merge_getnext (pstate=<value optimized out>)
at nodeGatherMerge.c:479
#3 ExecGatherMerge (pstate=<value optimized out>) at nodeGatherMerge.c:241
#4 0x00000000006251fe in ExecProcNode (aggstate=0x157a6d0)
at ../../../src/include/executor/executor.h:249
#5 fetch_input_tuple (aggstate=0x157a6d0) at nodeAgg.c:688
#6 0x0000000000629264 in agg_retrieve_direct (pstate=<value optimized out>)
at nodeAgg.c:2313
#7 ExecAgg (pstate=<value optimized out>) at nodeAgg.c:2124
#8 0x00000000006396ef in ExecProcNode (pstate=0x1579d98)
at ../../../src/include/executor/executor.h:249
#9 ExecNestLoop (pstate=0x1579d98) at nodeNestloop.c:160
#10 0x000000000061bc3f in ExecProcNode (queryDesc=0x14d5570,
direction=<value optimized out>, count=0, execute_once=-104 '\230')
at ../../../src/include/executor/executor.h:249
#11 ExecutePlan (queryDesc=0x14d5570, direction=<value optimized out>,
count=0, execute_once=-104 '\230') at execMain.c:1693
#12 standard_ExecutorRun (queryDesc=0x14d5570,
direction=<value optimized out>, count=0, execute_once=-104 '\230')
at execMain.c:362

regards, tom lane

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

#15Amit Kapila
amit.kapila16@gmail.com
In reply to: Tom Lane (#14)
hackersbugs
Re: [HACKERS] [postgresql 10 beta3] unrecognized node type: 90

On Thu, Aug 17, 2017 at 7:49 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:

Amit Kapila <amit.kapila16@gmail.com> writes:

On Tue, Aug 15, 2017 at 7:16 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:

I should think it wouldn't be that expensive to create a test
case, if you already have test cases that invoke GatherMerge.
Adding a right join against a VALUES clause with a small number of
entries, and a non-mergeable/hashable join clause, ought to do it.

I have done some experiments based on this idea to generate a test,
but I think it is not as straightforward as it appears.

I did this (the first 4 SETs duplicate what's already used in
select_parallel.sql):

regression=# set parallel_setup_cost=0;
SET
regression=# set parallel_tuple_cost=0;
SET
regression=# set min_parallel_table_scan_size=0;
SET
regression=# set max_parallel_workers_per_gather=4;
SET
regression=# set enable_hashagg TO 0;
SET
regression=# set enable_material TO 0;
SET
regression=# explain select * from (select string4, count((unique2))
from tenk1 group by string4 order by string4) ss right join
(values(1),(2)) v(x) on true;
QUERY PLAN
--------------------------------------------------------------------------------------------------
Nested Loop Left Join (cost=524.15..1086.77 rows=8 width=76)
-> Values Scan on "*VALUES*" (cost=0.00..0.03 rows=2 width=4)
-> Finalize GroupAggregate (cost=524.15..543.29 rows=4 width=72)
Group Key: tenk1.string4
-> Gather Merge (cost=524.15..543.17 rows=16 width=72)
Workers Planned: 4
-> Partial GroupAggregate (cost=524.10..542.89 rows=4 width=72)
Group Key: tenk1.string4
-> Sort (cost=524.10..530.35 rows=2500 width=68)
Sort Key: tenk1.string4
-> Parallel Seq Scan on tenk1 (cost=0.00..383.00 rows=2500 width=68)
(11 rows)

regression=# select * from (select string4, count((unique2))
from tenk1 group by string4 order by string4) ss right join
(values(1),(2)) v(x) on true;
server closed the connection unexpectedly

So, not only is it not that hard to reach ExecReScanGatherMerge,
but there is indeed a bug to fix there somewhere. The stack
trace indicates that the failure occurs in a later execution
of ExecGatherMerge:

This will be fixed by the patch [1]/messages/by-id/CAA4eK1JkByysFJNh9M349u_nNjqETuEnY_y1VUc_kJiU0bxtaQ@mail.gmail.com (execrescan_gathermerge_v2.patch)
I posted sometime back. The test case is slightly different, but may
I can re post the patch with your test case.

[1]: /messages/by-id/CAA4eK1JkByysFJNh9M349u_nNjqETuEnY_y1VUc_kJiU0bxtaQ@mail.gmail.com

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

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

#16Amit Kapila
amit.kapila16@gmail.com
In reply to: Amit Kapila (#15)
hackersbugs
Re: [BUGS] [postgresql 10 beta3] unrecognized node type: 90

On Thu, Aug 17, 2017 at 8:08 PM, Amit Kapila <amit.kapila16@gmail.com> wrote:

On Thu, Aug 17, 2017 at 7:49 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:

Amit Kapila <amit.kapila16@gmail.com> writes:

On Tue, Aug 15, 2017 at 7:16 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:

I should think it wouldn't be that expensive to create a test
case, if you already have test cases that invoke GatherMerge.
Adding a right join against a VALUES clause with a small number of
entries, and a non-mergeable/hashable join clause, ought to do it.

I have done some experiments based on this idea to generate a test,
but I think it is not as straightforward as it appears.

I did this (the first 4 SETs duplicate what's already used in
select_parallel.sql):

regression=# set parallel_setup_cost=0;
SET
regression=# set parallel_tuple_cost=0;
SET
regression=# set min_parallel_table_scan_size=0;
SET
regression=# set max_parallel_workers_per_gather=4;
SET
regression=# set enable_hashagg TO 0;
SET
regression=# set enable_material TO 0;
SET
regression=# explain select * from (select string4, count((unique2))
from tenk1 group by string4 order by string4) ss right join
(values(1),(2)) v(x) on true;
QUERY PLAN
--------------------------------------------------------------------------------------------------
Nested Loop Left Join (cost=524.15..1086.77 rows=8 width=76)
-> Values Scan on "*VALUES*" (cost=0.00..0.03 rows=2 width=4)
-> Finalize GroupAggregate (cost=524.15..543.29 rows=4 width=72)
Group Key: tenk1.string4
-> Gather Merge (cost=524.15..543.17 rows=16 width=72)
Workers Planned: 4
-> Partial GroupAggregate (cost=524.10..542.89 rows=4 width=72)
Group Key: tenk1.string4
-> Sort (cost=524.10..530.35 rows=2500 width=68)
Sort Key: tenk1.string4
-> Parallel Seq Scan on tenk1 (cost=0.00..383.00 rows=2500 width=68)
(11 rows)

regression=# select * from (select string4, count((unique2))
from tenk1 group by string4 order by string4) ss right join
(values(1),(2)) v(x) on true;
server closed the connection unexpectedly

So, not only is it not that hard to reach ExecReScanGatherMerge,
but there is indeed a bug to fix there somewhere. The stack
trace indicates that the failure occurs in a later execution
of ExecGatherMerge:

This will be fixed by the patch [1] (execrescan_gathermerge_v2.patch)
I posted sometime back. The test case is slightly different, but may
I can re post the patch with your test case.

I have kept the fix as it is but changed the test to match your test.
I think the another patch posted above to add a new guc for
enable_gather is still relevant and important.

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

Attachments:

execrescan_gathermerge_v3.patchapplication/octet-stream; name=execrescan_gathermerge_v3.patchDownload+50-0
#17Tom Lane
tgl@sss.pgh.pa.us
In reply to: Amit Kapila (#16)
hackersbugs
Re: [HACKERS] [postgresql 10 beta3] unrecognized node type: 90

Amit Kapila <amit.kapila16@gmail.com> writes:

I think the another patch posted above to add a new guc for
enable_gather is still relevant and important.

FWIW, I'm pretty much -1 on that. I don't see that it has any
real-world use; somebody who wants to turn that off would likely
really want to turn off parallelism altogether. We have
mucho knobs in that area already.

regards, tom lane

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

#18Tom Lane
tgl@sss.pgh.pa.us
In reply to: Amit Kapila (#16)
hackersbugs
Re: [HACKERS] [postgresql 10 beta3] unrecognized node type: 90

Amit Kapila <amit.kapila16@gmail.com> writes:

This will be fixed by the patch [1] (execrescan_gathermerge_v2.patch)
I posted sometime back. The test case is slightly different, but may
I can re post the patch with your test case.

I have kept the fix as it is but changed the test to match your test.

Pushed, with minor tidying of the test case. I think we can now
close this open item.

regards, tom lane

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

#19Tom Lane
tgl@sss.pgh.pa.us
In reply to: Tom Lane (#18)
hackersbugs
Re: [BUGS] [postgresql 10 beta3] unrecognized node type: 90

I wrote:

Pushed, with minor tidying of the test case. I think we can now
close this open item.

Nope, spoke too soon. See buildfarm.

(Man, am I glad I insisted on a test case.)

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

#20Robert Haas
robertmhaas@gmail.com
In reply to: Tom Lane (#19)
hackersbugs
Re: [HACKERS] [postgresql 10 beta3] unrecognized node type: 90

On Thu, Aug 17, 2017 at 2:06 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:

I wrote:

Pushed, with minor tidying of the test case. I think we can now
close this open item.

Nope, spoke too soon. See buildfarm.

(Man, am I glad I insisted on a test case.)

Whoa, that's not good.

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

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

#21Tom Lane
tgl@sss.pgh.pa.us
In reply to: Robert Haas (#20)
hackersbugs
#22Tom Lane
tgl@sss.pgh.pa.us
In reply to: Tom Lane (#21)
hackersbugs
#23Amit Kapila
amit.kapila16@gmail.com
In reply to: Tom Lane (#22)
hackersbugs
#24Tom Lane
tgl@sss.pgh.pa.us
In reply to: Tom Lane (#22)
hackersbugs
#25Tom Lane
tgl@sss.pgh.pa.us
In reply to: Tom Lane (#22)
hackersbugs
#26Amit Kapila
amit.kapila16@gmail.com
In reply to: Tom Lane (#25)
hackersbugs
#27Tom Lane
tgl@sss.pgh.pa.us
In reply to: Amit Kapila (#26)
hackersbugs
#28Amit Kapila
amit.kapila16@gmail.com
In reply to: Tom Lane (#27)
hackersbugs
#29Tom Lane
tgl@sss.pgh.pa.us
In reply to: Amit Kapila (#28)
hackersbugs
#30Amit Kapila
amit.kapila16@gmail.com
In reply to: Tom Lane (#29)
hackersbugs
#31Tom Lane
tgl@sss.pgh.pa.us
In reply to: Amit Kapila (#30)
hackersbugs
#32Robert Haas
robertmhaas@gmail.com
In reply to: Tom Lane (#31)
hackersbugs
#33Tom Lane
tgl@sss.pgh.pa.us
In reply to: Robert Haas (#32)
hackersbugs
#34Robert Haas
robertmhaas@gmail.com
In reply to: Tom Lane (#33)
hackersbugs
#35Tom Lane
tgl@sss.pgh.pa.us
In reply to: Robert Haas (#34)
hackersbugs
#36Robert Haas
robertmhaas@gmail.com
In reply to: Tom Lane (#35)
hackersbugs
#37Tom Lane
tgl@sss.pgh.pa.us
In reply to: Robert Haas (#36)
hackersbugs
#38Robert Haas
robertmhaas@gmail.com
In reply to: Tom Lane (#37)
hackersbugs
#39Amit Kapila
amit.kapila16@gmail.com
In reply to: Robert Haas (#38)
hackersbugs
#40Tom Lane
tgl@sss.pgh.pa.us
In reply to: Amit Kapila (#39)
hackersbugs
#41Amit Kapila
amit.kapila16@gmail.com
In reply to: Tom Lane (#40)
hackersbugs
#42Tom Lane
tgl@sss.pgh.pa.us
In reply to: Amit Kapila (#41)
hackersbugs
#43Robert Haas
robertmhaas@gmail.com
In reply to: Tom Lane (#42)
hackersbugs
#44Tom Lane
tgl@sss.pgh.pa.us
In reply to: Amit Kapila (#41)
hackersbugs