Re: Performance regression with PostgreSQL 11 and partitioning

From: David Rowley <david(dot)rowley(at)2ndquadrant(dot)com>
To: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Cc: Robert Haas <robertmhaas(at)gmail(dot)com>, Ashutosh Bapat <ashutosh(dot)bapat(at)enterprisedb(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-11 13:20:06
Message-ID: CAKJS1f9=BdwPKFo886SMwaC1-H_BrUyoaGP3GhrSXD2qPt=sXA@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On 9 June 2018 at 07:08, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> If we can
> replace those with something along the line of root->index_array[varno]
> we'll be better off across the board.

Are you talking about the loop over the appinfos in functions such as
adjust_appendrel_attrs_mutator?

If so, please note that it's looking for the appinfo with the parent
relid matching the appinfo, not a child relid. There may be many
appinfos in this "index_array" with that parent, so what you mention
above, if I understand you correctly, is not possible. We'd still need
a list of childrelids and we'd need to still loop over each
AppendRelInfo which belongs to that child and check if the parent
matches the Var.

I started coding a patch which instead of the direct lookup you've
mentioned just does a bms_next_member() loop using the given
childrelids. It only part way through writing the patch when I hit a
snag around the various places that call adjust_appendrel_attrs() with
a pre-determined AppendRelInfo, for example, the one in
inheritance_planner() at line 1312. I'd thought that maybe I could
just pass the childrelids in with
bms_make_singleton(appinfo->child_relid), but the manufactured
"parent_root" inside the inheritance_planner does not have the
append_rel_array set. We could work around this by having another
version of adjust_appendrel_attrs() which accepts an AppendRelInfo
instead of a Relids, but that looks like it means making another
adjust_appendrel_attrs_mutator like function and duplicating quite a
bit of code. Another option is perhaps to add a callback function to
adjust_appendrel_attrs_context to have that search for the
AppendRelInfo, or just return the known one, but unsure if that's
going to look very good. I've now stopped as I'm starting to think
that this is not improving the code.

I've attached the half done broken, regression test failing patch just
so you can see what I'm talking about.

--
David Rowley http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services

Attachment Content-Type Size
get_rid_of_find_appinfos_by_relids_incomplete_mess.patch application/octet-stream 24.3 KB

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Robert Haas 2018-06-11 13:27:17 Re: [PATCH v16] GSSAPI encryption support
Previous Message Ashutosh Bapat 2018-06-11 12:43:52 Re: TupleTableSlot abstraction