So what's going on here is that create_ordered_paths() does this:
foreach(lc, input_rel->pathlist)
{
Path *input_path = (Path *) lfirst(lc);
if (/* input path is suitably sorted already */)
sorted_path = input_path;
else
/* build a sorted path atop input_path */
/* Add projection step if needed */
if (sorted_path->pathtarget != target)
sorted_path = apply_projection_to_path(root, ordered_rel,
sorted_path, target);
add_path(ordered_rel, sorted_path);
}
Thus, if the input RelOptInfo has a path that already has the correct
ordering and output target, we'll try to add that path directly to
the output RelOptInfo. This is cheating in at least two ways:
1. The path's parent link isn't right: it still points to the input rel.
2. As per Jeevan's report, we now potentially have two different links
to the path. add_path could reject and free the path immediately,
or it could do so later while comparing it to some path offered later
for the output RelOptInfo, and either case leads to a dangling pointer
in the input RelOptInfo's pathlist.
Now, the reason we get away with #2 is that nothing looks at the lower
RelOptInfo's pathlist anymore after create_ordered_paths: we will only
be interested in paths that contribute to a surviving Path in the
output RelOptInfo, and those will be linked directly from the upper
Path. However, that's clearly kind of fragile, plus it's a bit
surprising that nobody has complained about #1.
We could probably fix this by creating a rule that you *must*
wrap a Path for a lower RelOptInfo into some sort of wrapper
Path before offering it as a candidate for an upper RelOptInfo.
(This could be cross-checked by having add_path Assert that
new_path->parent == parent_rel. The wrapper could be a do-nothing
ProjectionPath, perhaps.) But I think there are multiple places
taking similar shortcuts, so I'm a bit worried about how much overhead
we'll add for what seems likely to be only a debugging annoyance.
A low-cost fix perhaps could be to unlink the lower rel's whole
path list (set input_rel->pathlist = NIL, also zero the related
fields such as cheapest_path) once we've finished selecting the
paths we want for the upper rel. That's not great for debuggability
either, but maybe it's the most appropriate compromise.
I don't recall how clearly I understood this while writing the
upper-planner-pathification patch years ago. I think I did
realize the code was cheating, but if so I failed to document
it, so far as I can see.
regards, tom lane
