Re: speeding up planning with partitions

On Sun, Apr 28, 2019 at 8:10 AM Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> Amit Langote <Langote_Amit_f8(at)lab(dot)ntt(dot)co(dot)jp> writes:
> > Not sure if you'll like it but maybe we could ignore even regular
> > inheritance child targets that are proven to be empty (is_dummy_rel()) for
> > a given query during the initial SELECT planning. That way, we can avoid
> > re-running relation_excluded_by_constraints() a second time for *all*
> > child target relations.
>
> My thought was to keep traditional inheritance working more or less
> as it has. To do what you're suggesting, we'd have to move generic
> constraint-exclusion logic up into the RTE expansion phase, and I don't
> think that's a particularly great idea. I think what we should be
> doing is applying partition pruning (which is a very specialized form
> of constraint exclusion) during RTE expansion, then applying generic
> constraint exclusion in relation_excluded_by_constraints, and not
> examining partition constraints again there if we already used them.

Just to clarify, I wasn't suggesting that we change query_planner(),
but the blocks in inheritance_planner() that perform initial planning
as if the query was SELECT and gather child target relations from that
planner run; the following consecutive blocks:

/*
* Before generating the real per-child-relation plans, do a cycle of
* planning as though the query were a SELECT.
...
*/
{
PlannerInfo *subroot;
and:

/*----------
* Since only one rangetable can exist in the final plan, we need to make
* sure that it contains all the RTEs needed for any child plan.
...
child_appinfos = NIL;
old_child_rtis = NIL;
new_child_rtis = NIL;
parent_relids = bms_make_singleton(top_parentRTindex);
foreach(lc, select_appinfos)
{
AppendRelInfo *appinfo = lfirst_node(AppendRelInfo, lc);
RangeTblEntry *child_rte;

/* append_rel_list contains all append rels; ignore others */
if (!bms_is_member(appinfo->parent_relid, parent_relids))
continue;

/* remember relevant AppendRelInfos for use below */
child_appinfos = lappend(child_appinfos, appinfo);

I'm suggesting that we don't add the child relations that are dummy
due to constraint exclusion to child_appinfos. Maybe, we'll need to
combine the two blocks so that the latter can use the PlannerInfo
defined in the former to look up the child relation to check if dummy.

> > Do you want me to update my patch considering the above summary?
>
> Yes please.

I will try to get that done hopefully by tomorrow.

(On extended holidays that those of us who are in Japan have around
this time of year.)

> However, I wonder whether you're thinking differently in
> light of what you wrote in [1]:

Thanks for checking that thread.

> >>> Pruning in 10.2 works using internally generated partition constraints
> >>> (which for this purpose are same as CHECK constraints). With the new
> >>> pruning logic introduced in 11, planner no longer considers partition
> >>> constraint because it's redundant to check them in most cases, because
> >>> pruning would've selected the right set of partitions. Given that the new
> >>> pruning logic is still unable to handle the cases like above, maybe we
> >>> could change the planner to consider them, at least until we fix the
> >>> pruning logic to handle such cases.
>
> If we take that seriously then it would suggest not ignoring partition
> constraints in relation_excluded_by_constraints. However, I'm of the
> opinion that we shouldn't let temporary deficiencies in the
> partition-pruning logic drive what we do here. I don't think the set
> of cases where we could get a win by reconsidering the partition
> constraints is large enough to justify the cycles expended in doing so;
> and it'll get even smaller as pruning gets smarter.

Yeah, maybe we could away with that by telling users to define
equivalent CHECK constraints for corner cases like that although
that's not really great.

Thanks,
Amit