[sqlsmith] Failed assertion in parallel worker (ExecInitSubPlan)

Andreas Seltenreich <seltenreich@gmx.de> writes:

when fuzz testing master as of c1543a8, parallel workers trigger the
following assertion in ExecInitSubPlan every couple hours.
TRAP: FailedAssertion("!(list != ((List *) ((void *)0)))", File: "list.c", Line: 390)
Sample backtraces of a worker and leader below, plan of leader attached.
The collected queries don't seem to reproduce it.

Odd. My understanding of the restrictions on parallel query is that
anything involving a SubPlan ought not be parallelized; and this query
doesn't seem to get parallelized when I try it. It seems like what you're
seeing is that sometimes that restriction fails to be checked, but how
could that happen?

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

On Fri, May 6, 2016 at 8:45 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote:

Andreas Seltenreich <seltenreich@gmx.de> writes:

when fuzz testing master as of c1543a8, parallel workers trigger the
following assertion in ExecInitSubPlan every couple hours.
TRAP: FailedAssertion("!(list != ((List *) ((void *)0)))", File:

"list.c", Line: 390)

Sample backtraces of a worker and leader below, plan of leader attached.
The collected queries don't seem to reproduce it.

Odd. My understanding of the restrictions on parallel query is that
anything involving a SubPlan ought not be parallelized;

Subplan references are considered parallel-restricted, so parallel plan can
be generated if there are subplans in a query, but they shouldn't be pushed
to workers. I have tried a somewhat simpler example to see if we pushdown
anything parallel restricted to worker in case of joins and it turned out
there are cases when that can happen. Consider below example:

create or replace function parallel_func_select() returns integer
as $$
declare
ret_val int;
begin
ret_val := 1000;
return ret_val;
end;
$$ language plpgsql Parallel Restricted;

CREATE TABLE t1(c1, c2) AS SELECT g, repeat('x', 5) FROM
generate_series(1, 10000000) g;

CREATE TABLE t2(c1, c2) AS SELECT g, repeat('x', 5) FROM
generate_series(1, 1000000) g;

Explain Verbose SELECT t1.c1 + parallel_func_select(), t2.c1 FROM t1 JOIN
t2 ON t1.c1 = t2.c1;

QUERY PLAN

--------------------------------------------------------------------------------
--------
Gather (cost=32813.00..537284.53 rows=1000000 width=8)
Output: ((t1.c1 + parallel_func_select())), t2.c1
Workers Planned: 2
-> Hash Join (cost=31813.00..436284.53 rows=1000000 width=8)
Output: (t1.c1 + parallel_func_select()), t2.c1
Hash Cond: (t1.c1 = t2.c1)
-> Parallel Seq Scan on public.t1 (cost=0.00..95721.08
rows=4166608 w
idth=4)
Output: t1.c1, t1.c2
-> Hash (cost=15406.00..15406.00 rows=1000000 width=4)
Output: t2.c1
-> Seq Scan on public.t2 (cost=0.00..15406.00 rows=1000000
widt
h=4)
Output: t2.c1
(12 rows)

From the above output it is clear that parallel restricted function is
pushed down below gather node. I found that though we have have care fully
avoided to push pathtarget below GatherPath in apply_projection_to_path()
if pathtarget contains any parallel unsafe or parallel restricted clause,
but we are separately also trying to apply pathtarget to partialpath list
which doesn't seem to be the correct way even if it is required. I think
this has been added during parallel aggregate patch and it seems to me this
is not required after the changes related to GatherPath in
apply_projection_to_path().

After applying the attached patch, it avoids to add parallel restricted
clauses below gather path.

Now back to the original bug, if you notice in plan file attached in
original bug report, the subplan is pushed below Gather node in target
list, but not to immediate join, rather at one more level down to SeqScan
path. I am still not sure how it has manage to push the restricted clauses
to that down the level.

Thoughts?

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

On Sat, May 7, 2016 at 6:37 PM, Amit Kapila <amit.kapila16@gmail.com> wrote:

On Fri, May 6, 2016 at 8:45 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote:

Andreas Seltenreich <seltenreich@gmx.de> writes:

when fuzz testing master as of c1543a8, parallel workers trigger the
following assertion in ExecInitSubPlan every couple hours.
TRAP: FailedAssertion("!(list != ((List *) ((void *)0)))", File:

"list.c", Line: 390)

Sample backtraces of a worker and leader below, plan of leader

attached.

The collected queries don't seem to reproduce it.

Odd. My understanding of the restrictions on parallel query is that
anything involving a SubPlan ought not be parallelized;

Subplan references are considered parallel-restricted, so parallel plan

can be generated if there are subplans in a query, but they shouldn't be
pushed to workers. I have tried a somewhat simpler example to see if we
pushdown anything parallel restricted to worker in case of joins and it
turned out there are cases when that can happen. Consider below example:

From the above output it is clear that parallel restricted function is

pushed down below gather node. I found that though we have have care fully
avoided to push pathtarget below GatherPath in apply_projection_to_path()
if pathtarget contains any parallel unsafe or parallel restricted clause,
but we are separately also trying to apply pathtarget to partialpath list
which doesn't seem to be the correct way even if it is required. I think
this has been added during parallel aggregate patch and it seems to me this
is not required after the changes related to GatherPath in
apply_projection_to_path().

After applying the attached patch, it avoids to add parallel restricted

clauses below gather path.

Now back to the original bug, if you notice in plan file attached in

original bug report, the subplan is pushed below Gather node in target
list, but not to immediate join, rather at one more level down to SeqScan
path. I am still not sure how it has manage to push the restricted clauses
to that down the level.

On further analysis, I think I know what is going on in the original bug
report. We add the Vars (build_base_rel_tlists) and PlaceholderVars
(add_placeholders_to_base_rels()) to each relations (RelOptInfo) target
during qurey_planner and the Subplans are added as PlaceHolderVars in
target expressions. Now while considering whether a particular rel can be
parallel in set_rel_consider_parallel(), we don't check the target
expressions to allow the relation for parallelism. I think we can prohibit
the relation to be considered for parallelism if it's target expressions
contain any parallel restricted clause. Fix on those lines is attached
with this mail.

Thanks to Dilip Kumar for helping me in narrowing down this particular
problem. We were not able to generate the exact test, but I think the
above theory is sufficient to prove that it can cause a problem as seen in
the original bug report.

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

On Thu, May 12, 2016 at 7:46 AM, Amit Kapila <amit.kapila16@gmail.com> wrote:

On further analysis, I think I know what is going on in the original bug
report. We add the Vars (build_base_rel_tlists) and PlaceholderVars
(add_placeholders_to_base_rels()) to each relations (RelOptInfo) target
during qurey_planner and the Subplans are added as PlaceHolderVars in target
expressions. Now while considering whether a particular rel can be parallel
in set_rel_consider_parallel(), we don't check the target expressions to
allow the relation for parallelism. I think we can prohibit the relation to
be considered for parallelism if it's target expressions contain any
parallel restricted clause. Fix on those lines is attached with this mail.

Thanks to Dilip Kumar for helping me in narrowing down this particular
problem. We were not able to generate the exact test, but I think the above
theory is sufficient to prove that it can cause a problem as seen in the
original bug report.

I could be wrong, but I thought that the target list for an expression
would always contain only Vars at this stage. Non-default tlists get
injected at the end of scan/join planning. Am I wrong?

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

Robert Haas <robertmhaas@gmail.com> writes:

I could be wrong, but I thought that the target list for an expression
would always contain only Vars at this stage. Non-default tlists get
injected at the end of scan/join planning. Am I wrong?

Target list for a relation, you mean? See relation.h:

* reltarget - Default Path output tlist for this rel; normally contains
* Var and PlaceHolderVar nodes for the values we need to
* output from this relation.
* List is in no particular order, but all rels of an
* appendrel set must use corresponding orders.
* NOTE: in an appendrel child relation, may contain
* arbitrary expressions pulled up from a subquery!

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

On Thu, May 12, 2016 at 11:48 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote:

Robert Haas <robertmhaas@gmail.com> writes:

I could be wrong, but I thought that the target list for an expression
would always contain only Vars at this stage. Non-default tlists get
injected at the end of scan/join planning. Am I wrong?

Target list for a relation, you mean? See relation.h:

* reltarget - Default Path output tlist for this rel; normally contains
* Var and PlaceHolderVar nodes for the values we need to
* output from this relation.
* List is in no particular order, but all rels of an
* appendrel set must use corresponding orders.
* NOTE: in an appendrel child relation, may contain
* arbitrary expressions pulled up from a subquery!

Err, wow. That makes my head hurt. Can you explain why this case
only arises for appendrel children, and not for plain rels?

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

Robert Haas <robertmhaas@gmail.com> writes:

Target list for a relation, you mean? See relation.h:

* reltarget - Default Path output tlist for this rel; normally contains
* Var and PlaceHolderVar nodes for the values we need to
* output from this relation.
* List is in no particular order, but all rels of an
* appendrel set must use corresponding orders.
* NOTE: in an appendrel child relation, may contain
* arbitrary expressions pulled up from a subquery!

Err, wow. That makes my head hurt. Can you explain why this case
only arises for appendrel children, and not for plain rels?

Well, plain rels only output Vars ;-)

But consider an appendrel representing

(SELECT x+1 FROM t1 UNION ALL SELECT y+2 FROM t2) ss(a)

The RTE for ss will have a reltarget list containing just "a".
Once we pull up the subqueries, the reltarget lists for the two child
appendrel members will need to contain "x+1" and "y+2" in order to be
equivalent to the parent's reltarget list. See set_append_rel_size(),
which does that transformation.

This doesn't happen with ordinary subquery flattening because there
isn't a RelOptInfo corresponding to an ordinary subquery that's been
pulled up into the parent query.

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

On Thu, May 12, 2016 at 11:37 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:

Robert Haas <robertmhaas@gmail.com> writes:

Target list for a relation, you mean? See relation.h:

* reltarget - Default Path output tlist for this rel; normally

contains

* Var and PlaceHolderVar nodes for the values we need

to

* output from this relation.
* List is in no particular order, but all rels of an
* appendrel set must use corresponding orders.
* NOTE: in an appendrel child relation, may contain
* arbitrary expressions pulled up from a subquery!

Err, wow. That makes my head hurt. Can you explain why this case
only arises for appendrel children, and not for plain rels?

Well, plain rels only output Vars ;-)

Does this mean that base rels can't contain PlaceHolderVars? Isn't it
possible in below code:

query_planner()
{
..
/*
* Now distribute "placeholders" to base rels as needed. This has to be
* done after join removal because removal could change whether a
* placeholder is evaluatable at a base rel.
*/
add_placeholders_to_base_rels(root);
..
}

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

On Fri, May 13, 2016 at 9:43 AM, Amit Kapila <amit.kapila16@gmail.com>
wrote:

On Thu, May 12, 2016 at 11:37 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:

Robert Haas <robertmhaas@gmail.com> writes:

Target list for a relation, you mean? See relation.h:

* reltarget - Default Path output tlist for this rel; normally

contains

* Var and PlaceHolderVar nodes for the values we

need to

* output from this relation.
* List is in no particular order, but all rels of an
* appendrel set must use corresponding orders.
* NOTE: in an appendrel child relation, may contain
* arbitrary expressions pulled up from a subquery!

Err, wow. That makes my head hurt. Can you explain why this case
only arises for appendrel children, and not for plain rels?

Well, plain rels only output Vars ;-)

Does this mean that base rels can't contain PlaceHolderVars? Isn't it

possible in below code:

Here I want to ask base rels which are plain rels?

It might be that I am missing something, but if we debug the serial plan
for original query [1]select ref_68.is_trigger_deletable as c0, (select d from public.renamecolumnchild limit 1 offset 15) as c1, ref_68.is_trigger_insertable_into as c2, 3 as c3 from information_schema.role_udt_grants as ref_67 left join information_schema.views as ref_68 on (ref_67.grantee = ref_68.table_catalog ) where true limit 138; for which this issue is reported, we have noticed
that PlaceHolderVars that contain subplans are added to base rels for which
RTE kind is (RTE_RELATION).

[1]: select ref_68.is_trigger_deletable as c0, (select d from public.renamecolumnchild limit 1 offset 15) as c1, ref_68.is_trigger_insertable_into as c2, 3 as c3 from information_schema.role_udt_grants as ref_67 left join information_schema.views as ref_68 on (ref_67.grantee = ref_68.table_catalog ) where true limit 138;
public.renamecolumnchild limit 1 offset 15) as c1,
ref_68.is_trigger_insertable_into as c2, 3 as c3 from
information_schema.role_udt_grants as ref_67 left join
information_schema.views as ref_68 on (ref_67.grantee =
ref_68.table_catalog ) where true limit 138;

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

On Fri, May 13, 2016 at 10:31 AM, Amit Kapila <amit.kapila16@gmail.com>
wrote:

Here I want to ask base rels which are plain rels?

It might be that I am missing something, but if we debug the serial plan
for original query [1] for which this issue is reported, we have noticed
that PlaceHolderVars that contain subplans are added to base rels for which
RTE kind is (RTE_RELATION).

[1] - select ref_68.is_trigger_deletable as c0, (select d from

Finally, I could reproduce this issue, with just three tables.

If targetlist of top query has subquery references and if subquery is part
of OUTER join, then while pulling up the subquery,
it will create PlaceHolderVar, which will have expressions.

After applying prohibit_parallel_clause_below_rel_v1.patch, it is not
selecting parallel plan.
So now its clear that because of sub query pullup, we may get expression in
targetlist while creating single table path list. So we need to avoid
parallel plan if it contains expression.

Below is my test:
--------------------------

postgres=# CREATE TABLE t1(c1, c2) AS SELECT g, repeat('x', 5) FROM
postgres-# generate_series(1, 10000000) g;

SELECT 10000000
postgres=#
postgres=# CREATE TABLE t2(c1, c2) AS SELECT g, repeat('x', 5) FROM
postgres-# generate_series(1, 1000000) g;
SELECT 1000000
postgres=# CREATE TABLE t3(c1, c2) AS SELECT g, repeat('x', 5) FROM
generate_series(1, 1000000) g;
SELECT 1000000
postgres=# analyze t1;
ANALYZE
postgres=# analyze t2;
ANALYZE
postgres=# analyze t3;
ANALYZE

postgres=# explain select t1.c1, y.x from t1 left join (select (select
t3.c1 from t3 where t3.c1=t2.c1) as x from t2)y on (y.x=t1.c1);
QUERY PLAN

-------------------------------------------------------------------------------------
Hash Right Join (cost=319113.85..96991333895.85 rows=9999860 width=8)
Hash Cond: ((SubPlan 2) = t1.c1)
-> Gather (cost=1000.00..7460943906.00 rows=1000000 width=8)
Workers Planned: 2
-> Parallel Seq Scan on t2 (cost=0.00..7460842906.00 rows=416667
width=8)
SubPlan 1
-> Seq Scan on t3 (cost=0.00..17906.00 rows=1 width=4)
Filter: (c1 = t2.c1)
-> Hash (cost=154053.60..154053.60 rows=9999860 width=4)
-> Seq Scan on t1 (cost=0.00..154053.60 rows=9999860 width=4)
SubPlan 2
-> Seq Scan on t3 t3_1 (cost=0.00..17906.00 rows=1 width=4)
Filter: (c1 = t2.c1)
(13 rows)

postgres=# select t1.c1, y.x from t1 left join (select (select t3.c1 from
t3 where t3.c1=t2.c1) as x from t2)y on (y.x=t1.c1);
LOG: worker process: parallel worker for PID 109446 (PID 109483) was
terminated by signal 11: Segmentation fault
LOG: terminating any other active server processes
WARNING: terminating connection because of crash of another server process
DETAIL: The postmaster has commanded this server process to roll back the
current transaction and exit, because another server process exited
abnormally and possibly corrupted shared memory.
HINT: In a moment you should be able to reconnect to the database and
repeat your command.
WARNING: terminating connection because of crash of another server process
DETAIL: The postmaster has commanded this server process to roll back the
current transaction and exit, because another server process exited
abnormally and possibly corrupted shared memory.
HINT: In a moment you should be able to reconnect to the database and
repeat your command.
WARNING: terminating connection because of crash of another server process
DETAIL: The postmaster has commanded this server process to roll back the
current transaction and exit, because another server process exited
abnormally and possibly corrupted shared memory.
HINT: In a moment you should be able to reconnect to the database and
repeat your command.

--
Regards,
Dilip Kumar
EnterpriseDB: http://www.enterprisedb.com

Amit Kapila writes:

avoid_restricted_clause_below_gather_v1.patch
prohibit_parallel_clause_below_rel_v1.patch

I didn't observe any parallel worker related coredumps since applying
these. The same amount of testing done before applying them yielded
about a dozend.

Dilip Kumar writes:

So now its clear that because of sub query pullup, we may get expression in
targetlist while creating single table path list. So we need to avoid
parallel plan if it contains expression.

This sounds like a rather heavy restriction though…

regards,
Andreas

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

On Sun, May 22, 2016 at 9:32 PM, Andreas Seltenreich <seltenreich@gmx.de>
wrote:

Amit Kapila writes:

avoid_restricted_clause_below_gather_v1.patch
prohibit_parallel_clause_below_rel_v1.patch

I didn't observe any parallel worker related coredumps since applying
these. The same amount of testing done before applying them yielded
about a dozend.

Thanks for verification.

Dilip Kumar writes:

So now its clear that because of sub query pullup, we may get

expression in

targetlist while creating single table path list. So we need to avoid
parallel plan if it contains expression.

This sounds like a rather heavy restriction though…

I think what Dilip means by above statement is to avoid parallel plan if
target list contains parallel unsafe or restricted expressions. We already
restrict generation of parallel plans if qualification for a relation
contains such expressions (refer set_rel_consider_parallel()), so this
doesn't sound to be heavy restriction.

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

On Thu, May 12, 2016 at 11:37 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:

Robert Haas <robertmhaas@gmail.com> writes:

Target list for a relation, you mean? See relation.h:

* reltarget - Default Path output tlist for this rel; normally

contains

* Var and PlaceHolderVar nodes for the values we need

to

* output from this relation.
* List is in no particular order, but all rels of an
* appendrel set must use corresponding orders.
* NOTE: in an appendrel child relation, may contain
* arbitrary expressions pulled up from a subquery!

Err, wow. That makes my head hurt. Can you explain why this case
only arises for appendrel children, and not for plain rels?

Well, plain rels only output Vars ;-)

But consider an appendrel representing

(SELECT x+1 FROM t1 UNION ALL SELECT y+2 FROM t2) ss(a)

The RTE for ss will have a reltarget list containing just "a".
Once we pull up the subqueries, the reltarget lists for the two child
appendrel members will need to contain "x+1" and "y+2" in order to be
equivalent to the parent's reltarget list. See set_append_rel_size(),
which does that transformation.

I think this can also lead to an issue, as currently for child relations,
we just copy consider_parallel flag of parent. Now, if the child rel
target list is adjusted such that it contains parallel restricted
expression, then we are bound to fail. I think to handle append rel case
appropriately, we should compute the consider_parallel flag for each child
relation in set_append_rel_size() and then later when computing the flag
for parent rel, consider the flag for child rels (basically if any child
rel has consider_parallel as false, then set consider_parallel for parent
as false). To achieve this, we need to call set_rel_consider_parallel()
after calling set_rel_size().

Thoughts?

Just to summarize, apart from above issue, we have discussed two different
issues related to parallel query in this thread.
a. Push down of parallel restricted clauses to nodes below gather. Patch
to fix same is posted upthread [1]/messages/by-id/CAA4eK1Ky2=HsTsT4hmfL=EAL5rv0_t59tvWzVK9HQKvN6Dovkw@mail.gmail.com.
b. Wrong assumption that target list can only contain Vars. Patch to fix
same is posted upthread [2]/messages/by-id/CAA4eK1L-Uo=s4=0jvvVA51pj06u5WdDvSQg673yuxJ_Ja+x86Q@mail.gmail.com. Test which prove our assumption is wrong is
also posted upthread [3]/messages/by-id/CAFiTN-vzg5BkK6kAh3OMhvgRu-uJvkjz47ybtopMAfGJp=zWqA@mail.gmail.com.

I will add this issue in list of open issues for 9.6 @
https://wiki.postgresql.org/wiki/PostgreSQL_9.6_Open_Items

[1]: /messages/by-id/CAA4eK1Ky2=HsTsT4hmfL=EAL5rv0_t59tvWzVK9HQKvN6Dovkw@mail.gmail.com
/messages/by-id/CAA4eK1Ky2=HsTsT4hmfL=EAL5rv0_t59tvWzVK9HQKvN6Dovkw@mail.gmail.com
[2]: /messages/by-id/CAA4eK1L-Uo=s4=0jvvVA51pj06u5WdDvSQg673yuxJ_Ja+x86Q@mail.gmail.com
/messages/by-id/CAA4eK1L-Uo=s4=0jvvVA51pj06u5WdDvSQg673yuxJ_Ja+x86Q@mail.gmail.com
[3]: /messages/by-id/CAFiTN-vzg5BkK6kAh3OMhvgRu-uJvkjz47ybtopMAfGJp=zWqA@mail.gmail.com
/messages/by-id/CAFiTN-vzg5BkK6kAh3OMhvgRu-uJvkjz47ybtopMAfGJp=zWqA@mail.gmail.com

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

On Thu, May 26, 2016 at 05:08:31PM +0530, Amit Kapila wrote:

Just to summarize, apart from above issue, we have discussed two different
issues related to parallel query in this thread.
a. Push down of parallel restricted clauses to nodes below gather. Patch
to fix same is posted upthread [1].
b. Wrong assumption that target list can only contain Vars. Patch to fix
same is posted upthread [2]. Test which prove our assumption is wrong is
also posted upthread [3].

I will add this issue in list of open issues for 9.6 @
https://wiki.postgresql.org/wiki/PostgreSQL_9.6_Open_Items

[1] -
/messages/by-id/CAA4eK1Ky2=HsTsT4hmfL=EAL5rv0_t59tvWzVK9HQKvN6Dovkw@mail.gmail.com
[2] -
/messages/by-id/CAA4eK1L-Uo=s4=0jvvVA51pj06u5WdDvSQg673yuxJ_Ja+x86Q@mail.gmail.com
[3] -
/messages/by-id/CAFiTN-vzg5BkK6kAh3OMhvgRu-uJvkjz47ybtopMAfGJp=zWqA@mail.gmail.com

[Action required within 72 hours. This is a generic notification.]

The above-described topic is currently a PostgreSQL 9.6 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
9.6 open item, please let us know. Otherwise, please observe the policy on
open item ownership[1]/messages/by-id/20160527025039.GA447393@tornado.leadboat.com and send a status update within 72 hours 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 9.6rc1. Consequently, I will appreciate your
efforts toward speedy resolution. Thanks.

[1]: /messages/by-id/20160527025039.GA447393@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

On Sat, May 7, 2016 at 9:07 AM, Amit Kapila <amit.kapila16@gmail.com> wrote:

From the above output it is clear that parallel restricted function is
pushed down below gather node. I found that though we have have care fully
avoided to push pathtarget below GatherPath in apply_projection_to_path() if
pathtarget contains any parallel unsafe or parallel restricted clause, but
we are separately also trying to apply pathtarget to partialpath list which
doesn't seem to be the correct way even if it is required. I think this has
been added during parallel aggregate patch and it seems to me this is not
required after the changes related to GatherPath in
apply_projection_to_path().

Good catch. Committed.

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

On Thu, May 12, 2016 at 2:07 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:

Err, wow. That makes my head hurt. Can you explain why this case
only arises for appendrel children, and not for plain rels?

Well, plain rels only output Vars ;-)

Hmm. Dilip's example in
/messages/by-id/CAFiTN-vzg5BkK6kAh3OMhvgRu-uJvkjz47ybtopMAfGJp=zWqA@mail.gmail.com
seems to show that it's possible to end up with a targetlist
containing non-Vars even for a baserel. In that example,
pull_up_subqueries() injects t2 into the parent query with a target
list containing two items, one of which is a PlaceHolderVar
referencing the subplan for t3.

Unfortunately, that makes it a bit hard to avoid wasting cycles here.
Amit's patch in
/messages/by-id/CAA4eK1L-Uo=s4=0jvvVA51pj06u5WdDvSQg673yuxJ_Ja+x86Q@mail.gmail.com
is a bit brute force: it just checks the target list for every
relation for parallel-restricted constructs. In most cases, no
subqueries having been pulled up, it will find none, but it'll still
have to walk the whole target list to figure that out. If we had some
way of knowing that nothing was pulled up into the current rel, we
could avoid that. This seems to apply equally to baserels and
appendrels; both will commonly have a target list containing only
Vars, but both can end up with non-Vars in the target list after
subqueries are pulled up.

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

On Wed, Jun 1, 2016 at 9:35 PM, Noah Misch <noah@leadboat.com> wrote:

[Action required within 72 hours. This is a generic notification.]

The above-described topic is currently a PostgreSQL 9.6 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
9.6 open item, please let us know. Otherwise, please observe the policy on
open item ownership[1] and send a status update within 72 hours 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 9.6rc1. Consequently, I will appreciate your
efforts toward speedy resolution. Thanks.

Status update:

I'm working on it. I think there are several issues here, of which
I've committed a fix for just one. I hope to have them all fixed by
next Friday. If not, I'll send another update then.

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

On Sat, Jun 4, 2016 at 1:53 AM, Robert Haas <robertmhaas@gmail.com> wrote:

On Thu, May 12, 2016 at 2:07 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:

Err, wow. That makes my head hurt. Can you explain why this case
only arises for appendrel children, and not for plain rels?

Well, plain rels only output Vars ;-)

Hmm. Dilip's example in

/messages/by-id/CAFiTN-vzg5BkK6kAh3OMhvgRu-uJvkjz47ybtopMAfGJp=zWqA@mail.gmail.com

seems to show that it's possible to end up with a targetlist
containing non-Vars even for a baserel. In that example,
pull_up_subqueries() injects t2 into the parent query with a target
list containing two items, one of which is a PlaceHolderVar
referencing the subplan for t3.

Unfortunately, that makes it a bit hard to avoid wasting cycles here.
Amit's patch in

/messages/by-id/CAA4eK1L-Uo=s4=0jvvVA51pj06u5WdDvSQg673yuxJ_Ja+x86Q@mail.gmail.com

is a bit brute force: it just checks the target list for every
relation for parallel-restricted constructs. In most cases, no
subqueries having been pulled up, it will find none, but it'll still
have to walk the whole target list to figure that out. If we had some
way of knowing that nothing was pulled up into the current rel, we
could avoid that. This seems to apply equally to baserels and
appendrels;

That seems doable, as for such rels we can only have Vars and
PlaceHolderVars in targetlist. Basically, whenever we are adding
PlaceHolderVars to a relation, just remember that information and use it
later. The patch doing the same is attached with this mail. Now still,
this won't cover the case of ChildRels for an Append Relation as for that
we adjust target list separately in set_append_rel_size. I think we need
to deal with it separately.

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

On Mon, Jun 6, 2016 at 6:07 AM, Amit Kapila <amit.kapila16@gmail.com> wrote:

That seems doable, as for such rels we can only have Vars and
PlaceHolderVars in targetlist. Basically, whenever we are adding
PlaceHolderVars to a relation, just remember that information and use it
later. The patch doing the same is attached with this mail. Now still,
this won't cover the case of ChildRels for an Append Relation as for that we
adjust target list separately in set_append_rel_size. I think we need to
deal with it separately.

This approach looks pretty good to me. Here's a revised version of
your patch, with some renaming and other adjustments. I'm not sure
exactly what you're referring to in set_append_rel_size, but I did add
a line of code there that might be relevant.

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