Re: Performance regression with PostgreSQL 11 and partitioning

Robert Haas <robertmhaas(at)gmail(dot)com> writes:
> On Fri, May 25, 2018 at 1:53 PM, Amit Langote <amitlangote09(at)gmail(dot)com> wrote:
>> Seems here that we call find_appinfos_by_relids here for *all*
>> partitions, even if all but one partition may have been pruned. I
>> haven't studied this code in detail, but I suspect it might be
>> unnecessary, although I might be wrong.

> Uggh. It might be possible to skip it for dummy children. That would
> leave the dummy child rels generating a different pathtarget than the
> non-dummy children, but I guess if we never use them again maybe it
> wouldn't matter.

I think this line of thought is shooting the messenger. The core of the
problem here is that the appinfo list, which is a data structure designed
with the expectation of having a few dozen entries at most, has got four
thousand entries (or twenty thousand if you try Thomas' bigger case).
What we need, if we're going to cope with that, is something smarter than
linear searches.

I'm not exactly impressed with the design concept of
find_appinfos_by_relids in itself, either, as what that does is to
linearly search the appinfo list to produce ... an array, which also
has to be linearly searched. In this particular example, it seems
that all the output arrays have length 1; if they did not then we'd
be seeing the search loops in adjust_appendrel_attrs et al taking
up unreasonable amounts of time as well. (Not to mention the palloc
cycles and unreclaimed space eaten by said arrays.)

I'm inclined to think that we should flush find_appinfos_by_relids
altogether, along with these inefficient intermediate arrays, and instead
have the relevant places in adjust_appendrel_attrs call some function
defined as "gimme the AppendRelInfo for child rel A and parent rel B,
working directly from the PlannerInfo root data". That could use a hash
lookup when dealing with more than some-small-number of AppendRelInfos,
comparable to what we do with join_rel_list/join_rel_hash.

I also wonder whether it's really necessary to do a fresh search
for each individual Var, as I see is currently happening in
adjust_appendrel_attrs_mutator. At the very least we could expect
that there would be runs of requests for the same parent/child pair,
and avoid fresh list searches/hash probes when the answer is the
same as last time. But do we really have to leave that for the bottom
level of the recursion, rather than doing it once per expr at some higher
call level?

Maybe it's all right to decide that this rejiggering can be left
for v12 ... did we promise anyone that it's now sane to use thousands
of partitions?

regards, tom lane