On Thu, Aug 3, 2017 at 9:38 PM, Ashutosh Bapat <ashutosh.ba...@enterprisedb.com> wrote: > On Thu, Aug 3, 2017 at 2:10 AM, Robert Haas <robertmh...@gmail.com> wrote: >> On Mon, Jul 31, 2017 at 7:59 AM, Ashutosh Bapat >> <ashutosh.ba...@enterprisedb.com> wrote: >>> Adding AppendRelInfos to root->append_rel_list as and when they are >>> created would keep parent AppendRelInfos before those of children. But >>> that function throws away the AppendRelInfo it created when their are >>> no real children i.e. in partitioned table's case when has no leaf >>> partitions. So, we can't do that. Hence, I chose to change the API to >>> return the list of AppendRelInfos when the given RTE has real >>> children. >> >> So, IIUC, the case you're concerned about is when you have a hierarchy >> of only partitioned tables, with no plain tables. For example, if B >> is a partitioned table and a partition of A, and that's all there is, >> A will recurse to B and B will return NIL. >> >> Is it necessary to get rid of the extra AppendRelInfos, or are they >> harmless like the duplicate RTE and PlanRowMark nodes? >> > > Actually there are two sides to this: > > If there are no leaf partitions, without the patch two things happen > 1. rte->inh is cleared and 2 no appinfo is added to the > root->append_rel_list, even though harmless RTE and PlanRowMark nodes > are created. The first avoids treating the relation as the inheritance > parent and thus avoids creating any child relations and paths, saving > a lot of work. Ultimately set_rel_size() marks such a relation as > dummy > 352 else if (rte->relkind == RELKIND_PARTITIONED_TABLE) > 353 { > 354 /* > 355 * A partitioned table without leaf > partitions is marked > 356 * as a dummy rel. > 357 */ > 358 set_dummy_rel_pathlist(rel); > 359 } > > Since root->append_rel_list is traversed for every inheritance parent, > not adding needless AppendRelInfos improves performance and saves > memory, (FWIW or consider a case where there are thousands of > partitioned partitions without any leaf partition.).
With some testing, I found that this was true once, but not after declarative partition support. Please check [1]. > > My initial thought was to keep both these properties intact. But then > removing such AppendRelInfos would have a problem when such a table is > on the inner side of the join as described in [1]. So I wrote the > patch not to do either of those things when there are partitioned > partitions without leaf partitions. So, it looks like you are correct, > we could just go ahead and add those AppendRelInfos directly to > root->append_rel_list. Irrespective of [1], I have implemented your idea of not changing signature of expand_inherited_rtentry() with following idea. >> >> If we do need to get rid of the extra AppendRelInfos, maybe a less >> invasive solution would be to change the if (!need_append) case to do >> root->append_rel_list = list_truncate(root->append_rel_list, >> original_append_rel_length). [1] https://www.postgresql.org/message-id/CAFjFpReWJr1yTkHU=oqimbmcycmosw3vpr39rbuq_ovwdfb...@mail.gmail.com -- Best Wishes, Ashutosh Bapat EnterpriseDB Corporation The Postgres Database Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers