On 12 May 2016 at 07:04, Robert Haas <robertmh...@gmail.com> wrote: > On Wed, May 11, 2016 at 1:57 PM, Robert Haas <robertmh...@gmail.com> wrote: >> I don't immediately understand what's going wrong here. It looks to >> me like make_group_input_target() already called, and that worked OK, >> but now make_partialgroup_input_target() is failing using more-or-less >> the same logic. Presumably that's because make_group_input_target() >> was called on final_target as returned by create_pathtarget(root, >> tlist), but make_partialgroup_input_target() is being called on >> grouping_target, which I'm guessing came from >> make_window_input_target, which somehow lacks sortgroupref labeling. >> But I don't immediately see how that would happen, so there's >> obviously something I'm missing here. > > So, it turns out you can reproduce this bug pretty easily without > force_parallel_mode, like this: > > alter table int4_tbl set (parallel_degree = 4); > SELECT SUM(COUNT(f1)) OVER () FROM int4_tbl WHERE f1=42; > > Or you can just a query that involves a window function on a table > large enough for parallelism to be considered: > > SELECT SUM(COUNT(aid)) OVER () FROM pgbench_accounts; > > The crash goes way if the target list involves at least one plain > column that uses no aggregate or window function, because then the > PathTarget list has a sortgrouprefs array. Trivial fix patch > attached, although some more review from you (Tom Lane, PathTarget > inventor and planner whiz) and David Rowley (author of this function) > would be appreciated in case there are deeper issues here.
The problem is make_group_input_target() only calls add_column_to_pathtarget() (which allocates this array) when there's a GROUP BY, otherwise it just appends to the non_group_col list. Since your query has no GROUP BY it means that add_column_to_pathtarget() is never called with a non-zero sortgroupref. It looks like Tom has intended that PathTarget->sortgrouprefs can be NULL going by both the comment /* corresponding sort/group refnos, or 0 */, and the coding inside add_column_to_pathtarget(), which does not allocate the array if given a 0 sortgroupref. It looks like make_sort_input_target(), make_window_input_target() and make_group_input_target() all get away without this check because they're all using final_target, which was built by make_pathtarget_from_tlist() which *always* allocates the sortgrouprefs array, even if it's left filled with zeros. It might be better if this was all consistent. Perhaps it would be worth modifying make_pathtarget_from_tlist() to only allocate the sortgrouprefs array if there's any non-zero tle->ressortgroupref, then modify the other make_*_input_target() functions to handle a NULL array, similar to the fix that's in your patch. This saves an allocation which is likely much more expensive than the NULL check later. Alternatively add_column_to_pathtarget() could be modified to allocate the array even if sortgroupref is zero. I think consistency is good here, as if this had been consistent this would not be a bug. -- David Rowley http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training & Services
