Thread: [sqlsmith] Crash in apply_projection_to_path

Hi,

the following query against the regression database crashes master as of
23b09e15.

select 1 from depth0 inner join depth1 on (depth0.c = depth1.c)        where depth0.c @@ depth1.c limit 1;

regards,
Andreas

Program terminated with signal SIGSEGV, Segmentation fault.
#0  create_projection_path (root=root@entry=0x6918e60,   rel=0x7f7f7f7f7f7f7f7f, subpath=0x69d6de8,
target=target@entry=0x69d7428)  at pathnode.c:2160
 
2160        pathnode->path.parallel_safe = rel->consider_parallel &&
#0  create_projection_path (root=root@entry=0x6918e60, rel=0x7f7f7f7f7f7f7f7f, subpath=0x69d6de8,
target=target@entry=0x69d7428)at pathnode.c:2160
 
#1  0x000000000067841e in apply_projection_to_path (root=0x6918e60, rel=0x69d5e18, path=0x69d6ee0, target=0x69d7428) at
pathnode.c:2251
#2  0x000000000065ff20 in grouping_planner (root=0x22e1850, root@entry=0x6918e60, inheritance_update=56 '8',
inheritance_update@entry=0'\000', tuple_fraction=2615.2579411764709, tuple_fraction@entry=0) at planner.c:1737
 
#3  0x0000000000661f44 in subquery_planner (glob=glob@entry=0x6918dc8, parse=parse@entry=0x235e3b0,
parent_root=parent_root@entry=0x0,hasRecursion=hasRecursion@entry=0 '\000', tuple_fraction=tuple_fraction@entry=0) at
planner.c:758
#4  0x0000000000662e2b in standard_planner (parse=0x235e3b0, cursorOptions=256, boundParams=0x0) at planner.c:307
#5  0x00000000006f40ed in pg_plan_query (querytree=0x235e3b0, cursorOptions=256, boundParams=0x0) at postgres.c:798
#6  0x00000000006f41e4 in pg_plan_queries (querytrees=<optimized out>, cursorOptions=256, boundParams=0x0) at
postgres.c:857
#7  0x00000000006f5c93 in exec_simple_query (query_string=<optimized out>) at postgres.c:1022

Andreas Seltenreich <seltenreich@gmx.de> writes:
> the following query against the regression database crashes master as of
> 23b09e15.

> select 1 from depth0 inner join depth1 on (depth0.c = depth1.c)
>          where depth0.c @@ depth1.c limit 1;

What's going on here is that add_partial_path is recycling a now-dominated
partial path without regard for the fact that there's already a GatherPath
pointing at that old partial path.  Later use of the GatherPath tries to
make use of its now-dangling subpath pointer.

I'd be inclined to think that it's silly to build GatherPaths in advance
of having finalized the list of partial paths for a rel.
        regards, tom lane

On Thu, Apr 28, 2016 at 10:06 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> Andreas Seltenreich <seltenreich@gmx.de> writes:
>> the following query against the regression database crashes master as of
>> 23b09e15.
>
>> select 1 from depth0 inner join depth1 on (depth0.c = depth1.c)
>>          where depth0.c @@ depth1.c limit 1;
>
> What's going on here is that add_partial_path is recycling a now-dominated
> partial path without regard for the fact that there's already a GatherPath
> pointing at that old partial path.  Later use of the GatherPath tries to
> make use of its now-dangling subpath pointer.
>
> I'd be inclined to think that it's silly to build GatherPaths in advance
> of having finalized the list of partial paths for a rel.

Uh ... yeah, that shouldn't be happening.  I obviously screwed something up.

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

Amit Kapila <amit.kapila16@gmail.com> writes:
>> On Thu, Apr 28, 2016 at 10:06 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>>> I'd be inclined to think that it's silly to build GatherPaths in advance
>>> of having finalized the list of partial paths for a rel.

> What's happening here is that to form joinrel, we need to call
> add_paths_to_joinrel() with both outer and inner relation twice, once with
> rel1 as outer relation and rel1 as inner relation and vice versa.  So now
> the second call to add_paths_to_joinrel() can replace a partial path which
> is being referenced by GatherPath generated in first call.  I think we
> should generate gather paths for join rel after both the calls
> to add_paths_to_joinrel() aka in make_join_rel().  Attached patch on above
> lines fixes the problem for me.

make_join_rel is certainly not far enough down the call stack to solve
this problem.  It can, and typically will, be invoked multiple times
for the same target join relation.

One possible answer is to do it in standard_join_search, just before
the set_cheapest call for each join relation.  You'd need to account
for the issue in GEQO search as well.
        regards, tom lane

Amit Kapila <amit.kapila16@gmail.com> writes:
> On Fri, Apr 29, 2016 at 7:33 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> make_join_rel is certainly not far enough down the call stack to solve
>> this problem.  It can, and typically will, be invoked multiple times
>> for the same target join relation.
>> 
>> One possible answer is to do it in standard_join_search, just before
>> the set_cheapest call for each join relation.

> Yes, that makes sense to me.

Done that way.

> Yet, another idea could be to create a copy of partial path before passing
> it to create_gather_path()?

That's just a kluge, and I'm not exactly convinced it'd solve the problem
anyway, unless you wanted to recursively copy the entire infrastructure
of the partial path.  What's needed is to set rules for how we build paths
up in an orderly fashion.  Such rules already existed for regular paths
(see the comments I added in c45bf5751), and now they exist for parallel
paths as well.
        regards, tom lane