On Fri, 9 Aug 2019 at 09:55, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> Attached is a patch that removes simple_rte_array in favor of just
> accessing the query's rtable directly. I concluded that there was
> not much point in messing with simple_rel_array or append_rel_array,
> because they are not in fact just mirrors of some List. There's no
> List at all of baserel RelOptInfos, and while we do have a list of
> AppendRelInfos, it's a compact, random-order list not one indexable
> by child relid.
>
> Having done this, though, I'm a bit discouraged about whether to commit
> it. In light testing, it's not any faster than HEAD and in complex
> queries seems to actually be a bit slower. I suspect the reason is
> that we've effectively replaced
> root->simple_rte_array[i]
> with
> root->parse->rtable->elements[i-1]
> and the two extra levels of indirection are taking their toll.
If there are no performance gains from this then -1 from me. We're
all pretty used to it the way it is
> I realized that it's pretty dumb not to
> just merge setup_append_rel_array into setup_simple_rel_arrays.
> The division of labor there is inconsistent with the fact that
> there's no such division in expand_planner_arrays.
ha, yeah I'd vote for merging those. It was coded that way originally
until someone objected! :)
> I still have hopes for getting rid of es_range_table_array though,
> and will look at that tomorrow or so.
Yes, please. I've measured that to be quite an overhead with large
partitioning setups. However, that was with some additional code which
didn't lock partitions until it was ... well .... too late... as it
turned out. But it seems pretty good to remove code that could be a
future bottleneck if we ever manage to do something else with the
locking of all partitions during UPDATE/DELETE.
--
David Rowley http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services