Re: speeding up planning with partitions

From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: Amit Langote <Langote_Amit_f8(at)lab(dot)ntt(dot)co(dot)jp>
Cc: David Rowley <david(dot)rowley(at)2ndquadrant(dot)com>, Imai Yoshikazu <yoshikazu_i443(at)live(dot)jp>, "jesper(dot)pedersen(at)redhat(dot)com" <jesper(dot)pedersen(at)redhat(dot)com>, "Imai, Yoshikazu" <imai(dot)yoshikazu(at)jp(dot)fujitsu(dot)com>, Amit Langote <amitlangote09(at)gmail(dot)com>, Alvaro Herrera <alvherre(at)2ndquadrant(dot)com>, Robert Haas <robertmhaas(at)gmail(dot)com>, Justin Pryzby <pryzby(at)telsasoft(dot)com>, Pg Hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: speeding up planning with partitions
Date: 2019-03-28 22:38:04
Message-ID: 23856.1553812684@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox
Thread:
Lists: pgsql-hackers

I've been hacking on this pretty hard over the last couple days,
because I really didn't like the contortions you'd made to allow
inheritance_planner to call expand_inherited_rtentry in a completely
different context than the regular code path did. I eventually got
rid of that by having inheritance_planner run one cycle of planning
the query as if it were a SELECT, and extracting the list of unpruned
children from that. I had to rearrange the generation of the final
rtable a bit to make that work, but I think that inheritance_planner
winds up somewhat cleaner and safer this way; it's making (slightly)
fewer assumptions about how much the results of planning the child
queries can vary.

Perhaps somebody will object that that's one more planning pass than
we had before, but I'm not very concerned, because
(a) at least for partitioned tables that we can prune successfully,
this should still be better than v11, since we avoid the planning
passes for pruned children.
(b) inheritance_planner is horridly inefficient anyway, in that it
has to run a near-duplicate planning pass for each child table.
If we're concerned about its cost, we should be working to get rid of
the function altogether, as per [1]. In the meantime, I do not want
to contort other code to make life easier for inheritance_planner.

There's still some loose ends:

1. I don't like 0003 much, and omitted it from the attached.
I think that what we ought to be doing instead is not having holes
in the rel_parts[] arrays to begin with, ie they should only include
the unpruned partitions. If we are actually encoding any important
information in those array positions, I suspect that is broken anyway
in view of 898e5e329: we can't assume that the association of child
rels with particular PartitionDesc slots will hold still from planning
to execution.

2. I seriously dislike what's been done in joinrels.c, too. That
really seems like a kluge (and I haven't had time to study it
closely).

3. It's not entirely clear to me why the patch has to touch
execPartition.c. That implies that the planner-to-executor API
changed, but how so, and why is there no comment update clarifying it?

Given the short amount of time left in this CF, there may not be
time to address the first two points, and I won't necessarily
insist that those be changed before committing. I'd like at least
a comment about point 3 though.

Attached is updated patch as a single patch --- I didn't think the
division into multiple patches was terribly helpful, due to the
flapping in expected regression results.

regards, tom lane

[1] https://www.postgresql.org/message-id/357.1550612935@sss.pgh.pa.us

Attachment Content-Type Size
speed-up-planning-with-partitions-38.patch text/x-diff 86.4 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Jeremy Schneider 2019-03-28 22:45:57 Re: Syntax diagrams in user documentation
Previous Message Christopher Browne 2019-03-28 22:35:20 Re: Syntax diagrams in user documentation