Re: Postgres_fdw join pushdown - INNER - FULL OUTER join combination generating wrong result

From: Etsuro Fujita <fujita(dot)etsuro(at)lab(dot)ntt(dot)co(dot)jp>
To: Ashutosh Bapat <ashutosh(dot)bapat(at)enterprisedb(dot)com>
Cc: Rajkumar Raghuwanshi <rajkumar(dot)raghuwanshi(at)enterprisedb(dot)com>, pgsql-hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: Postgres_fdw join pushdown - INNER - FULL OUTER join combination generating wrong result
Date: 2016-04-15 12:00:34
Message-ID: 5710D7E2.7010302@lab.ntt.co.jp
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On 2016/04/14 15:20, Ashutosh Bapat wrote:
> On Thu, Apr 14, 2016 at 8:42 AM, Etsuro Fujita
> <fujita(dot)etsuro(at)lab(dot)ntt(dot)co(dot)jp <mailto:fujita(dot)etsuro(at)lab(dot)ntt(dot)co(dot)jp>> wrote:

> As you mentioned, we could
> support FULL JOIN fully, by encapsulating a joining relation with
> conditions into a subquery. And ISTM that it is relatively easy to
> do that, by borrowing some ideas from Hanada-san's original join
> pushdown patch. If it's OK, I'll create a patch for that in a few days.

> In his patch the deparsing targetlist and conditions required that the
> FROM clause entries were ready with the columns from base relations and
> joins realiased. That's no more true. We deparse every Var node as
> <relation alias>.<column name> where relation alias is nothing but rN; N
> being index of RangeTblEntry. So, Hanada-san's method to deparse
> recursively can not be applied as such now.

I think so, too. I don't think his ideas could be applied as is.

> Here's what needs to be done:
> When we identify that certain relation (base or join) needs a subquery
> to be deparsed (because the join relation above it could not pull the
> quals up), we remember it in the upper join relation. Before deparsing
> 1. we walk the join tree and collect targetlists of all such relations,
> 2. associate column aliases with those targetlists (save the column
> alises in resname?) and craft a relation alias 3. associate the
> relations alias, column aliases and targetlists with the base relations
> involved in such relations (may be creating a list similar to rtable).
> While deparsing a Var node, we check if corresponding base relation is
> itself or part of a relation deparsed as a subquery. If it is then we
> lookup that Var in the targetlist associated with the base relation and
> use corresponding relation and column alias for deparsing it. Otherwise,
> deparse it as <relation alias>.<column name> usually.

Good to know. That is what I have in mind, except for the way of
collecting subqueries' columns and associating those columns with
relation and column aliases, which I think can be done more easily.
Please find attached a WIP patch. That patch works well at least for
queries in your patch. Maybe I'm missing something, though.

> That looks like a big code change to go after feature freeze. So, we
> will leave it for 9.7, unless RMT allows us to introduce that change.

OK

Best regards,
Etsuro Fujita

Attachment Content-Type Size
pg_fj_cond_efujita.patch text/x-patch 20.8 KB

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Michael Paquier 2016-04-15 12:08:11 Re: Rework help interface of vcregress.pl
Previous Message Feike Steenbergen 2016-04-15 11:48:24 Parallel indicators not written by pg_get_functiondef