On Wed, Sep 29, 2010 at 11:01 AM, Robert Haas <robertmh...@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

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

Attachment: merge_append_2010_09_29.patch
Description: Binary data

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

Reply via email to