From: Etsuro Fujita <fujita.etsuro@lab.ntt.co.jp>
To: Tom Lane <tgl@sss.pgh.pa.us>
Cc: srinivasan.sa@zohocorp.com
Cc: pgsql-bugs@lists.postgresql.org
Subject: Re: BUG #15613: Bug in PG Planner for Foreign Data Wrappers
Date: Wed, 06 Feb 2019 13:15:49 +0900
Message-ID: <5C5A5F75.5040203@lab.ntt.co.jp> (raw)
In-Reply-To: <19209.1548957996@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>
<5C52C769.5090600@lab.ntt.co.jp>
<19209.1548957996@sss.pgh.pa.us>
(2019/02/01 3:06), Tom Lane wrote:
> Etsuro Fujita<fujita.etsuro@lab.ntt.co.jp> writes:
>> (2019/01/31 2:48), Tom Lane wrote:
>>> So I see two alternatives for fixing this aspect of the problem:
>>>
>>> 1. Just change file_fdw and postgres_fdw as above, and hope that
>>> authors of extension FDWs get the word.
>>>
>>> 2. Modify create_foreignscan_path so that it doesn't simply trust
>>> required_outer to be correct, but forcibly adds rel->lateral_relids
>>> into it. This would fix the problem without requiring FDWs to be
>>> on board, but it seems kinda grotty, and it penalizes FDWs that
>>> have gone to the trouble of computing the right required_outer relids
>>> in the first place. (It's somewhat amusing that postgres_fdw
>>> appears to get this right when it generates parameterized paths,
>>> but not in the base case.)
>
>> #2 seems like a good idea, as it would make FDW authors' life easy.
>
> I started to fix this, and soon noticed what seems a worse problem:
> postgres_fdw is using create_foreignscan_path to construct Paths for
> join relations and upperrels. This is utterly broken. That function
> was only designed to produce paths for baserels, which is why it uses
> get_baserel_parampathinfo. At the very least we're getting wrong
> rowcount estimates for parameterized joinrels (compare
> get_joinrel_parampathinfo), and it seems possible that we're actually
> getting wrong plans with the wrong set of movable join clauses being
> applied. And I have no idea what might go wrong for upperrels, though
> I think those are never parameterized so it might accidentally not fail.
Ah, you are right. I also noticed that when I proposed parameterized
foreign joins for postgres_fdw two years ago, but I forgot that.
> We could either split the function into two or three functions, or add
> still more overhead to it to notice what kind of relation has been
> passed and adjust its behavior for that. I'm not really thrilled with
> the latter: the fact that it's called create_foreignSCAN_path means,
> to me, that it's not supposed to be used for anything but baserel
> cases.
I don't have any strong opinion on that.
> I think one big question here is how many external FDWs may have
> copied postgres_fdw's remote-join handling. If we just have to
> fix postgres_fdw, my thoughts about what to do are probably
> different than if we have to try to avoid making third-party callers
> more broken than they are already.
As far as I know oracle_fdw supports join pushdown the same way as
postgres_fdw [1], but other than that, I guess there are few if any.
Best regards,
Etsuro Fujita
[1] http://laurenz.github.io/oracle_fdw/
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-bugs@postgresql.org
Cc: fujita.etsuro@lab.ntt.co.jp, tgl@sss.pgh.pa.us, srinivasan.sa@zohocorp.com, pgsql-bugs@lists.postgresql.org
Subject: Re: BUG #15613: Bug in PG Planner for Foreign Data Wrappers
In-Reply-To: <5C5A5F75.5040203@lab.ntt.co.jp>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox