On Wed, Sep 29, 2010 at 11:01 AM, Robert Haas <robertmhaas@gmail.com> wrote:
> Makes sense to me. It seems that there are actually two halves to
> this problem: getting the child EMs to be generated in the first
> place, and then getting them to be examined at the appropriate time.
So I tried out the logic described in this email and, with a few
modifications, it seemed to work. Updated patch attached, any review
appreciated. There are still a bunch of other things that need to be
fixed here, but I think this is OK as far as this particular issue is
concerned. I fixed a few other things:
- create_append_plan must call create_plan_recurse rather than
create_plan. This appears to be an error introduced by rebasing; the
previous version of the patch seg faults when attempting to execute a
plan wherein an inner index scan has been pushed down through an
append node
- remove the hack to short-circuit the append node altogether if
there's only one child
- some miscellaneous code cleanup (more is needed)
> 3. TGL: "Speaking of sorting, it's not entirely clear to me how the
> patch ensures that all the child plans produce the necessary sort keys
> as output columns, and especially not how it ensures that they all get
> produced in the *same* output columns. This might accidentally manage
> to work because of the "throwaway" call to make_sort_from_pathkeys(),
> but at the very least that's a misleading comment." I'm not sure what
> needs to be done about this; I'm going to look at this further.
I spent some time looking at this complaint, and I'm still not sure
what needs to be done about it. Experimentation reveals that the
neither the throwaway call to make_sort_from_pathkeys() nor any other
call to that function is creating the relevant target list entries.
It appears that they're getting created when set_append_rel_pathlist()
calls adjust_appendrel_attrs(). I'm not sure whether that's
sufficient or not. make_sort_from_pathkeys() could add any missing
entries later, but I'm not too sure the order would match if that
happened.
Another awkwardness of this patch is that it makes
create_append_path() and consequently set_dummy_rel_pathlist() take an
additional "root" argument. While there's nothing terribly
unreasonable about this on its face, it's only necessary so that
create_append_path() can call cost_sort(), which takes "root" but
doesn't actually use it. I'm not sure whether it's better to leave
this as-is or to remove the root argument from cost_sort().
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise Postgres Company