Compute attr_needed for child relations (was Re: inherit support for foreign tables)

From: Etsuro Fujita <fujita(dot)etsuro(at)lab(dot)ntt(dot)co(dot)jp>
To: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Cc: Ashutosh Bapat <ashutosh(dot)bapat(at)enterprisedb(dot)com>, Kyotaro HORIGUCHI <horiguchi(dot)kyotaro(at)lab(dot)ntt(dot)co(dot)jp>, Shigeru Hanada <shigeru(dot)hanada(at)gmail(dot)com>, pgsql-hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Compute attr_needed for child relations (was Re: inherit support for foreign tables)
Date: 2014-08-14 04:35:46
Message-ID: 53EC3CA2.9050506@lab.ntt.co.jp
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

(2014/08/08 18:51), Etsuro Fujita wrote:
>>> (2014/06/30 22:48), Tom Lane wrote:
>>>> I wonder whether it isn't time to change that. It was coded like that
>>>> originally only because calculating the values would've been a waste of
>>>> cycles at the time. But this is at least the third place where it'd be
>>>> useful to have attr_needed for child rels.

> I've revised the patch.

There was a problem with the previous patch, which will be described
below. Attached is the updated version of the patch addressing that.

The previous patch doesn't cope with some UNION ALL cases properly. So,
e.g., the server will crash for the following query:

postgres=# create table ta1 (f1 int);
CREATE TABLE
postgres=# create table ta2 (f2 int primary key, f3 int);
CREATE TABLE
postgres=# create table tb1 (f1 int);
CREATE TABLE
postgres=# create table tb2 (f2 int primary key, f3 int);
CREATE TABLE
postgres=# explain verbose select f1 from ((select f1, f2 from (select
f1, f2, f3 from ta1 left join ta2 on f1 = f2 limit 1) ssa) union all
(select f1,
f2 from (select f1, f2, f3 from tb1 left join tb2 on f1 = f2 limit 1)
ssb)) ss;

With the updated version, we get the right result:

postgres=# explain verbose select f1 from ((select f1, f2 from (select
f1, f2, f3 from ta1 left join ta2 on f1 = f2 limit 1) ssa) union all
(select f1,
f2 from (select f1, f2, f3 from tb1 left join tb2 on f1 = f2 limit 1)
ssb)) ss;
QUERY PLAN
--------------------------------------------------------------------------------
Append (cost=0.00..0.05 rows=2 width=4)
-> Subquery Scan on ssa (cost=0.00..0.02 rows=1 width=4)
Output: ssa.f1
-> Limit (cost=0.00..0.01 rows=1 width=4)
Output: ta1.f1, (NULL::integer), (NULL::integer)
-> Seq Scan on public.ta1 (cost=0.00..34.00 rows=2400
width=4)
Output: ta1.f1, NULL::integer, NULL::integer
-> Subquery Scan on ssb (cost=0.00..0.02 rows=1 width=4)
Output: ssb.f1
-> Limit (cost=0.00..0.01 rows=1 width=4)
Output: tb1.f1, (NULL::integer), (NULL::integer)
-> Seq Scan on public.tb1 (cost=0.00..34.00 rows=2400
width=4)
Output: tb1.f1, NULL::integer, NULL::integer
Planning time: 0.453 ms
(14 rows)

While thinking to address this problem, Ashutosh also expressed concern
about the UNION ALL handling in the previous patch in a private email.
Thank you for the review, Ashutosh!

Thanks,

Best regards,
Etsuro Fujita

Attachment Content-Type Size
attr_needed-v3.patch text/x-diff 9.5 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message MauMau 2014-08-14 04:52:37 Re: [patch] pg_copy - a command for reliable WAL archiving
Previous Message Pavel Stehule 2014-08-14 04:29:19 proposal: allow to specify result tupdesc and mode in SPI API