Re: BUG #15613: Bug in PG Planner for Foreign Data Wrappers

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: srinivasan(dot)sa(at)zohocorp(dot)com, pgsql-bugs(at)lists(dot)postgresql(dot)org
Subject: Re: BUG #15613: Bug in PG Planner for Foreign Data Wrappers
Date: 2019-02-06 04:15:49
Message-ID: 5C5A5F75.5040203@lab.ntt.co.jp
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-bugs

(2019/02/01 3:06), Tom Lane wrote:
> Etsuro Fujita<fujita(dot)etsuro(at)lab(dot)ntt(dot)co(dot)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/

In response to

Responses

Browse pgsql-bugs by date

  From Date Subject
Next Message Amit Langote 2019-02-06 07:35:09 Re: 'update returning *' returns 0 columns instead of empty row with 2 columns when (i) no rows updated and (ii) when applied to a partitioned table with sub-partition
Previous Message Bruce Momjian 2019-02-05 16:51:17 Re: BUG #15620: pgAdmin IV pgAgent multi-schedule jobs messed up view of schedules