Re: [HACKERS] Removing [Merge]Append nodes which contain a single subpath

From: David Rowley <david(dot)rowley(at)2ndquadrant(dot)com>
To: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Cc: PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: [HACKERS] Removing [Merge]Append nodes which contain a single subpath
Date: 2017-11-15 06:17:28
Views: Raw Message | Whole Thread | Download mbox
Lists: pgsql-hackers

On 1 November 2017 at 02:47, David Rowley <david(dot)rowley(at)2ndquadrant(dot)com> wrote:
> On 27 October 2017 at 04:47, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
>> Have you considered turning AppendPath with a single child into more
>> of a special case? I think this is morally equivalent to your ProxyPath
>> proposal, but it would take less new boilerplate code to support.
>> If you taught create_append_path to inherit the child's pathkeys
>> when there's only one child, and taught createplan.c to skip making the
>> useless Append node, I think you might be nearly done.

I've made another pass over this patch today to try to ensure it's in
a better state. I'm away on leave until the end of the month starting
from this Friday, so likely I won't get any time to resolve any issues
found until maybe the last day of the month.

I've attached an updated version of the patch which fixes a bug that I
discovered today where I had neglected to translate the quals to a
grouping set. I noticed this when I made all the main regression
tables partitioned tables containing a single child partition with
MINVALUE to MAXVALUE range. The fact that all plans now appear to work
as planned after having removed the Append has given me a bit more
confidence that I've not missed any places to translate Vars, but I'm
still not 100% certain. I just can't think of any other way or testing
that at the moment. Any that I have missed should result in an error
like "variable not found in subplan target list"

I've also modified the patch to make use of some existing functions in
prepunion.c which gather up AppendRelInfos for a childrel. I'd
previously invented my own function because the existing ones were not
quite exactly what I needed. I'd planned to come back to it, but only
just got around to fixing that.

Apart from that, the changes are just in the code comments.

The remove_singleton_appends_examples_of_differences_2017-11-15.patch
which I've attached applies changes to the regression tests to make
many of the major tables partitioned tables with a single partition.
It also changes the expected results of the small insignificant plan
changes and leaves only the actual true differences. This file is not
intended for commit, but more just a demo of it working for a larger
variety of plan shapes. There is actually no additional tests with the
patch. It relies on the places in the existing tests which have
changed. I didn't add any extra as I can't think of any particular
area to target given that it's such a generic thing that can apply to
so many different cases.

David Rowley
PostgreSQL Development, 24x7 Support, Training & Services

Attachment Content-Type Size
remove_singleton_appends_2017-11-15.patch application/octet-stream 49.2 KB
remove_singleton_appends_examples_of_differences_2017-11-15.patch application/octet-stream 101.5 KB

In response to


Browse pgsql-hackers by date

  From Date Subject
Next Message Beena Emerson 2017-11-15 06:49:42 Re: [HACKERS] Runtime Partition Pruning
Previous Message Connor Wolf 2017-11-15 06:00:46 Re: [HACKERS] How to implement a SP-GiST index as a extension module?