From: Tom Lane <tgl@sss.pgh.pa.us>
To: Etsuro Fujita <fujita.etsuro@lab.ntt.co.jp>
cc: srinivasan.sa@zohocorp.com, pgsql-bugs@lists.postgresql.org
Subject: Re: BUG #15613: Bug in PG Planner for Foreign Data Wrappers
In-reply-to: <24678.1548870484@sss.pgh.pa.us>
References: <15613-092be1be9576c728@postgresql.org>
 <5C51993C.2070606@lab.ntt.co.jp> <19832.1548865557@sss.pgh.pa.us>
 <24678.1548870484@sss.pgh.pa.us>
Comments: In-reply-to Tom Lane <tgl@sss.pgh.pa.us>
	message dated "Wed, 30 Jan 2019 12:48:04 -0500"
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-ID: <32028.1548878858.1@sss.pgh.pa.us>
Date: Wed, 30 Jan 2019 15:07:38 -0500
Message-ID: <32029.1548878858@sss.pgh.pa.us>
Precedence: bulk

I wrote:
> The source of all the problems seems to be that the IS NOT NULL
> variable is getting wrapped into a PlaceHolderVar incorrectly(?):
> by the time it gets to distribute_qual_to_rels, it looks like
> ...
> I haven't tracked down why that's happening, but this parsetree
> clearly says that Var 1/2 (that is, ref_0.t1_col2) should get
> evaluated at the scan of RTE 6 (ref_1), which would then make
> ref_1 be the natural place to do the filter check too.

Ah: that is expected after all.  I hadn't looked closely enough at the
structure of the test query, but what the IS NOT NULL is supposed to test
is actually a lateral reference in the tlist of the ref_1 scan (subq_0).
That's underneath an outer join that could potentially force it to null,
so it needs to be evaluated under the outer join, and the PlaceHolderVar
is inserted to ensure that that happens.

Of course, since the outer join gets strength-reduced to a plain join,
we didn't really need to do any such thing.  But PlaceHolderVar insertion
happens during pull_up_subqueries, which runs before reduce_outer_joins,
so the where-to-evaluate decision is made first.  (And that explains why
the bug goes away if we manually reduce the join type.)

Switching the order of those two steps would not be an improvement,
either.  In this example, for instance, the fact that we can reduce
the outer join depends on the WHERE IS NOT NULL test, which initially
is one subquery level above the outer join, so if we hadn't flattened
the subqueries we'd fail to prove that we could reduce the outer join.

In short, while this could theoretically be better it's pretty hard
to see how we could make it so, short of a vast and complex rewrite.
And the case is weird enough that it seems unlikely we'd be improving
very many real-world queries.  (I'm betting, given what the test case
looks like, that this is a sqlsmith find rather than an anonymization
of a real query.)

So we're down to one bug: the FDWs are being careless about including
lateral_relids in their path parameterization.

			regards, tom lane