Re: Performance regression with PostgreSQL 11 and partitioning

From: Ashutosh Bapat <ashutosh(dot)bapat(at)enterprisedb(dot)com>
To: David Rowley <david(dot)rowley(at)2ndquadrant(dot)com>
Cc: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, Robert Haas <robertmhaas(at)gmail(dot)com>, Amit Langote <amitlangote09(at)gmail(dot)com>, Thomas Reiss <thomas(dot)reiss(at)dalibo(dot)com>, "pgsql-hackers(at)postgresql(dot)org" <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: Performance regression with PostgreSQL 11 and partitioning
Date: 2018-06-05 04:44:57
Message-ID: CAFjFpRd+hZmJ1iqqC4Gv5TKz8DQ1GFrxJ7-0WDQQ9Fyitmv9iw@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Tue, Jun 5, 2018 at 5:50 AM, David Rowley
<david(dot)rowley(at)2ndquadrant(dot)com> wrote:
> On 5 June 2018 at 01:35, Ashutosh Bapat <ashutosh(dot)bapat(at)enterprisedb(dot)com> wrote:
>> I was wondering if we can get rid of append_rel_list altogether. In
>> your patch, you have saved AppendRelInfos by child_relid. So all the
>> slots indexed by parent relid are empty. We could place AppendRelInfos
>> by parent relid. Thus a given AppendRelInfo would be places at two
>> places, one at the index of child relid and second at the index
>> pointed by parent relid. That's fine even in case of multilevel
>> inheritance since an intermediate parent has two relids one as a
>> parent and other as a child.
>>
>> One problem with that we do not know how long the array would be to
>> start with. But we could keep on repallocing the array to increase its
>> size.
>
> To be honest, the patch was meant as a discussion topic for ways to
> improve the reported regression in PG11. The patch was put together
> fairly quickly.

I think the idea is brilliant. I do not have objections for trying
something in that direction. I am suggesting that we take this a bit
forward and try to eliminate append_rel_list altogether.

>
> I've not really considered what happens in the case where an inherited
> table has multiple parents. The patch happens to pass regression
> tests, but I've not checked to see if the existing tests create any
> tables this way.

AFAIK, that case doesn't arise while processing a query. Examining the
way we create AppendRelInfos during expand_inherited_tables(), it's
clear that we create only one AppendRelInfo for a given child and also
for a given child and parent pair. If there are two tables from which
a child inherits, the child's RTE/RelOptInfo is associated only with
the top-most parent that appears in the query. See following comment
from find_all_inheritors()
/*
* Add to the queue only those children not already seen. This avoids
* making duplicate entries in case of multiple inheritance paths from
* the same parent. (It'll also keep us from getting into an infinite
* loop, though theoretically there can't be any cycles in the
* inheritance graph anyway.)
*/

>
> Perhaps it's okay to change things this way for just partitioned
> tables and leave inheritance hierarchies alone.
>

There's no point having two separate code paths when internally we are
treating them as same. The only difference between partitioning and
inheritance is that for multi-level partitioned table, we preserve the
inheritance hierarchy whereas for multi-level inheritance, we flatten
it.

--
Best Wishes,
Ashutosh Bapat
EnterpriseDB Corporation
The Postgres Database Company

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Tom Lane 2018-06-05 04:55:13 Re: Code of Conduct plan
Previous Message Pavel Stehule 2018-06-05 04:32:31 Re: plans for PostgreSQL 12