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-01-28 20:00:08
Message-ID: 20140128200008.GA2135209@tornado.leadboat.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Tue, Jan 28, 2014 at 06:53:32PM +0900, Kyotaro HORIGUCHI wrote:
> Hello, Thank you, and I' sorry to have kept you waiting.

No hurry.

> > > The attached two patches are rebased to current 9.4dev HEAD and
> > > make check at the topmost directory and src/test/isolation are
> > > passed without error. One bug was found and fixed on the way. It
> > > was an assertion failure caused by probably unexpected type
> > > conversion introduced by collapse_appendrels() which leads
> > > implicit whole-row cast from some valid reltype to invalid
> > > reltype (0) into adjust_appendrel_attrs_mutator().
> >
> > What query demonstrated this bug in the previous type 2/3 patches?

I would still like to know the answer to the above question.

> > [transvar_merge_mutator()] has roughly the same purpose as
> > adjust_appendrel_attrs_mutator(), but it propagates the change to far fewer
> > node types. Why this case so much simpler? The parent translated_vars of
> > parent_appinfo may bear mostly-arbitrary expressions.
>
> There are only two places where AppendRelInfo node is generated,
> expand_inherited_rtentry and
> pull_up_union_leaf_queries.(_copyAppendrelInfo is irrelevant to
> this discussion.) The core part generating translated_vars for
> them are make_inh_translation_list and
> make_setop_translation_list respectively. That's all. And they
> are essentially works on the same way, making a Var for every
> referred target entry of children like following.
>
> | makeVar(varno,
> | tle->resno,
> | exprType((Node *) tle->expr),
> | exprTypmod((Node *) tle->expr),
> | exprCollation((Node *) tle->expr),
> | 0);

It's true that each AppendRelInfo is initially created that way, but they need
not remain so simple. You can assume ctx->child_appinfo->translated_vars
contains only these Var nodes, but parent_appinfo->translated_vars may contain
arbitrary expressions. (I think the pullup_replace_vars() call at
prepjointree.c:954 is where an AppendRelInfo can get non-Var expressions.)

> So all we should do to collapse nested appendrels is simplly
> connecting each RTEs directly to the root (most ancient?) RTE in
> the relationship, resolving Vars like above, as seen in patched
> expand_inherited_rtentry.
>
> # If translated_vars are generated always in the way shown above,
> # using tree walker might be no use..
>
> This can be done apart from all other stuffs compensating
> translation skew done in adjust_appendrel_attrs. I believe they
> are in orthogonal relationship.

Here is a test case that provokes an assertion failure under the patch; I
believe the cause is oversimplification in transvars_merge_mutator():

create table parent (a int, b int);
create table child () inherits (parent);
select parent from parent union all select parent from parent;

While poking at that test case, I noticed that the following test emits three
rows on HEAD and wrongly emits four rows with the patch:

create table parent (a int, b int);
create table child () inherits (parent);
insert into parent values (1,10);
insert into child values (2,20);
select a, b from parent union all select a, b from child;

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

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Jeff Janes 2014-01-28 20:03:22 Re: Fwd: Request for error explaination || Adding a new integer in indextupleData Structure
Previous Message Josh Berkus 2014-01-28 19:56:40 Re: proposal: hide application_name from other users