On 2018-Jun-27, David Rowley wrote: > I've only just completed reading back through all the code and I think > it's correct. I ended up changing add_paths_to_append_rel() so that > instead of performing concatenation on partitioned_rels from two UNION > ALL children, it creates a List of lists. I believe this list can > only end up with a 2-level hierarchy of partitioned rels. I tested > this and it seems to work as I expect, although I think I need to > study the code a bit more to ensure it can't happen. I need to check > if there's some cases where nested UNION ALLs cannot be flattened to > have a single UNION ALL parent. Supporting this did cause me to have > to check the List->type in a few places. I only saw one other place in > the code where this is done, so I figured that meant it was okay.
Checking a node's ->type member is not idiomatic -- use IsA() for that. (Strangely, we have IsIntegerList() but only for use within list.c.) But do we rely on the ordering of these lists anywhere? I'm wondering why it's not more sensible to use a bitmapset there (I guess for your list-of-lists business you'd have a list of bms's). I didn't look your patch much further. Since Tom has been revamping this code lately, I think it's a good idea to wait for his input. -- Álvaro Herrera https://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
