Re: [HACKERS] Removing [Merge]Append nodes which contain a single subpath
От | David Rowley |
---|---|
Тема | Re: [HACKERS] Removing [Merge]Append nodes which contain a single subpath |
Дата | |
Msg-id | CAKJS1f9eHBEykHLsvN-Tn5SwH+GBJzWmrSq=qS1S7rw9c+mvkw@mail.gmail.com обсуждение исходный текст |
Ответ на | Re: [HACKERS] Removing [Merge]Append nodes which contain a single subpath (Tom Lane <tgl@sss.pgh.pa.us>) |
Ответы |
Re: [HACKERS] Removing [Merge]Append nodes which contain a single subpath
|
Список | pgsql-hackers |
On 27 October 2017 at 04:47, Tom Lane <tgl@sss.pgh.pa.us> wrote: > David Rowley <david.rowley@2ndquadrant.com> writes: >> It seems like a good idea for the planner not to generate Append or >> MergeAppend paths when the node contains just a single subpath. >> ... >> Perhaps there is also a "Method 3" which I've not thought about which >> would be much cleaner. > > 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. That seems reasonable, although after going and implementing this, it does not seem quite as convenient as dummy paths are. I had to make the AppendPath carries a bit more weight than I originally thought. It now stores translate to/from expressions and also a single bool (isproxy) to mark if it's a proxy type Append. I tried to do without this bool and just check if there's a single subpath and some translation expressions, but that was getting fooled by Paths which had empty targetlists which meant there were no translations, and going on the single subpath alone is no good as do we create append paths more places than just add_paths_to_append_rel(). I've attached a patch which goes and implements all this. I think it still needs a bit of work. I'm not 100% confident I'm translating expressions in all the required places in createplan.c. I did modify tenk1 to become the parent of a single partition and ran the regression tests. There appear to be 4 subtle differences to querying a parent with an only-child vs querying the child directly. 1. EXPLAIN VERBOSE shows the columns prefixed by the table name. This is caused by "useprefix = list_length(es->rtable) > 1;" in explain.c. 2. There's a small change in an error message in portals.out. "ERROR: cursor "c" is not a simply updatable scan of table "tenk1a"" now mentions the child table instead of the parent. Nothing to worry about as it appears to already do this with native partitioning. 3. Some plans are parallelized that were not before (see join.out). This is caused by compute_parallel_worker() having a special case for non-baserels. 4. Some gating plans don't work the same as they used to, this is highlighted in aggregates.out and is caused by child rel Const TRUE baserestrictinfo clauses being dropped in set_append_rel_size. For differences 2-4, I've attached another file which modifies tenk1 to become a partitioned table with a single partition. I've modified all the expected results for #1 to reduce the noise, so 3 tests are still failing for reasons 2-4. I'm not at all concerned with #2. #3 might not be worth fixing, I've just no idea how yet and #4 seems like it's on purpose. Other things I'm not 100% happy with are; in prepunion.c I had to invent a new function named find_all_appinfos_for_child, which is a bit similar to adjust_appendrel_attrs_multilevel, only it modifies the attributes, but the use I have for find_all_appinfos_for_child includes also calling add_child_rel_equivalences for the children at each level. Probably a bit more thinking will come up with some way to modify the existing function sets to be more reusable, I've just not done that yet. I decided that in createplan.c to have a generic expression translator rather than something that only understands AppendRelInfos. The reason for this is two-fold: 1. There may be multiple AppendRelInfos required for a single translation between a parent and child if there are multiple other children in between. Seems wasteful to have intermediate translations. 2. I have other uses in mind for this feature that are not Append child relations. A sanity check on what I have would be most welcome if anyone has time. I'm going to add this to the November commitfest. -- David Rowley http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training & Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Вложения
В списке pgsql-hackers по дате отправления:
Следующее
От: Tom LaneДата:
Сообщение: Re: [HACKERS] Re: PANIC: invalid index offnum: 186 when processing BRIN indexes in VACUUM