Re: UNION ALL on partitioned tables won't use indices.

From: Noah Misch <noah(at)leadboat(dot)com>
To: Kyotaro HORIGUCHI <horiguchi(dot)kyotaro(at)lab(dot)ntt(dot)co(dot)jp>
Cc: tgl(at)sss(dot)pgh(dot)pa(dot)us, peter_e(at)gmx(dot)net, robertmhaas(at)gmail(dot)com, pgsql-hackers(at)postgresql(dot)org
Subject: Re: UNION ALL on partitioned tables won't use indices.
Date: 2014-03-03 17:16:17
Message-ID: 20140303171617.GE3446923@tornado.leadboat.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Mon, Mar 03, 2014 at 07:01:09PM +0900, Kyotaro HORIGUCHI wrote:
> > > Yes, the old dumped version of typ2 patch did so. It flattened
> > > appendrel tree for the query prpoerly. Let me hear the reson you
> > > prefer to do so.
> >
> > Having reviewed my upthread reasoning for preferring one of those two
> > approaches over the other, it's a weak preference. They have similar runtime
> > costs. Putting the logic with the code that creates appendrels reduces the
> > number of code sites one must examine to reason about possible plan
> > structures. We might not flatten RTE_RELATION appendrel parents exactly the
> > same way we flatten RTE_SUBQUERY appendrel parents. I would tend to leave
> > inh=true for the former, for reasons explained in my notes on v7, but set
> > inh=false for the latter to save needless work.
>
> Unfortunately, RTE_SUBQUERY-es with their inh flag cleard might
> cause something inconvenient in preprocess_minmax_aggregates()
> and/or add_rtes_to_flat_rtable()..

preprocess_minmax_aggregates() only cares about RTEs reachable from the join
tree, and the appendrel parents made obsolete by flattening would not be
reachable from the join tree. add_rtes_to_flat_rtable() might be unhappy.

> # I haven't found that related to sepgsql, though :-(
>
> I understood that your concern is to deal parent RTEs defferently
> according to their relkinds. But I think the inh flags could not
> be modified at all for the reason mentioned above.

That's fine, then. It was a minor point.

If you are convinced that a separate flattening pass is best, that suffices
for me at this stage. Please submit the patch you have in mind, incorporating
any improvements from the v7 patch that are relevant to both approaches.

> At least as of now, inheritance tables define the bottom bound of
> a appendrel tree and on the other hand complex(?) union_alls
> define the upper bound, and both multilevel (simple)union_alls
> and inheritances are flattened individually so all possible
> inheritance tree to be collapsed by this patch is only, I think,
>
> Subquery(inh=1)[Relation-inhparent [Relation-child, child, child]]
>
> > On the other hand, a flattening pass is less code overall and
> > brings an attractive uniformity-by-default to the area.
>
> Focusing only on the above structure, what we should do to
> collapse this tree is only connect the childs to the Subquery
> directly, then remove all appendrelinfos connecting to the
> Relation-inhparent. inh flag need not to be modified.
>
> # Umm. I strongly suspect that I overlooked something..
>
> Then even widening to general case, the case doesn't seem to
> change. All we need to do is, link a child to its grandparent and
> isolate the parent removing apprelinfos.

I barely follow what you're saying here. Do you speak of
union_inh_idx_typ2_v2_20131113.patch, unionall_inh_idx_typ3_v7.patch, or some
future design? If we use a separate flattening pass, there's no small limit
on how many layers of appendrel we may need to flatten. pull_up_subqueries()
can create many nested RTE_SUBQUERY appendrel layers; there may be more than
just child, parent and grandparent. There's never more than one layer of
RTE_RELATION appendrel, though.

Thanks,
nm

--
Noah Misch
EnterpriseDB http://www.enterprisedb.com

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Alvaro Herrera 2014-03-03 17:18:35 Re: patch: option --if-exists for pg_dump
Previous Message Stephen Frost 2014-03-03 17:08:26 Re: GSoC proposal - "make an unlogged table logged"