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

From: Kyotaro HORIGUCHI <horiguchi(dot)kyotaro(at)lab(dot)ntt(dot)co(dot)jp>
To: noah(at)leadboat(dot)com
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-02-03 10:36:22
Message-ID: 20140203.193622.14506008.horiguchi.kyotaro@lab.ntt.co.jp
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Hello, The attached file
"unionall_inh_idx_typ3_v6_20140203.patch" is the new version of
this patch fixed the 'whole-row-var' bug.

First of all, on second thought about this,

> > 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;
>
> Mmm. I had the same result. Please let me have a bit more time.

This turned out to be a correct result. The two tables have
following records after the two INSERTs.

| =# select * from only parent;
| 1 | 10
| (1 row)
|
| =# select * from child;
| 2 | 20
| (1 row)

Then it is natural that the parent-side in the UNION ALL returns
following results.

| =# select * from parent;
| a | b
| ---+----
| 1 | 10
| 2 | 20
| (2 rows)

Finally, the result we got has proved not to be a problem.

====
Second, about the crash in this sql,

> select parent from parent union all select parent from parent;

It is ignored whole-row reference (=0) which makes the index of
child translated-vars list invalid (-1). I completely ignored it
in spite that myself referred to before.

Unfortunately ConvertRowtypeExpr prevents appendrels from being
removed currently, and such a case don't seem to take place so
often, so I decided to exclude the case. In addition, I found
that only setting "rte->inh = false" causes duplicate scanning on
the same relations through abandoned appendrels so I set
parent/child_relid to InvalidOid so as no more to referred to
from the ex-parent (and ex-children).

The following queries seems to work correctly (but with no
optimization) after these fixes.

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 parent from parent union all select parent from parent;
parent
--------
(1,10)
(2,20)
(1,10)
(2,20)
(4 rows)
select a, parent from parent union all select a,parent from parent;
a | parent
---+--------
1 | (1,10)
2 | (2,20)
1 | (1,10)
2 | (2,20)
(4 rows)
select a, b from parent union all select a, b from parent;
a | b
---+----
1 | 10
2 | 20
1 | 10
2 | 20
(4 rows)
select a, b from parent union all select a, b from child;
a | b
---+----
2 | 20
1 | 10
2 | 20
(3 rows)

> > > > > 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.

I rememberd after some struggles. It failed during 'make check',
on the following query in inherit.sql.

| update bar set f2 = f2 + 100
| from
| ( select f1 from foo union all select f1+3 from foo ) ss
| where bar.f1 = ss.f1;

The following SQLs causes the same type of crash.

create temp table foo(f1 int, f2 int);
create temp table foo2(f3 int) inherits (foo);
create temp table bar(f1 int, f2 int);
update bar set f2 = 1
from
( select f1 from foo union all select f1+3 from foo ) ss
where bar.f1 = ss.f1;

The tipped stone was "wholerow1" for the subquery created in
preprocess_targetlist.

| /* Not a table, so we need the whole row as a junk var */
| var = makeWholeRowVar(rt_fetch(rc->rti, range_table),
..
| snprintf(resname, sizeof(resname), "wholerow%u", rc->rowmarkId);

Then the finishing blow was a appendRelInfo corresponding to the
var above, whose parent_reltype = 0 and child_reltype != 0, and
of course introduced by my patch. Since such a situation takes
place even for this patch, the modification is left alone.

regards,

--
Kyotaro Horiguchi
NTT Open Source Software Center

Attachment Content-Type Size
unionall_inh_idx_typ3_v6_20140203.patch text/x-patch 7.9 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Andres Freund 2014-02-03 10:37:01 Re: narwhal and PGDLLIMPORT
Previous Message Christian Kruse 2014-02-03 10:06:35 Re: Patch: Show process IDs of processes holding a lock; show relation and tuple infos of a lock to acquire