Re: postgres_fdw: wrong results with self join + enable_nestloop off

From: Önder Kalacı <onderkalaci(at)gmail(dot)com>
To: Etsuro Fujita <etsuro(dot)fujita(at)gmail(dot)com>
Cc: Richard Guo <guofenglinux(at)gmail(dot)com>, Nishant Sharma <nishant(dot)sharma(at)enterprisedb(dot)com>, Suraj Kharage <suraj(dot)kharage(at)enterprisedb(dot)com>, pgsql-hackers(at)postgresql(dot)org
Subject: Re: postgres_fdw: wrong results with self join + enable_nestloop off
Date: 2023-08-15 14:02:41
Message-ID: CACawEhV=+Q0HXrcDergbTR9EkVFukgRPMTZbRFL-YK5CRmvYag@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Hi Etsuro, all

The commit[1] seems to break some queries in Citus[2], which is an
extension which relies on set_join_pathlist_hook.

Although the comment says */*Finally, give extensions a chance to
manipulate the path list.*/ *we use it to extract lots of information
about the joins and do the planning based on the information.

Now, for some joins where consider_join_pushdown=false, we cannot get the
information that we used to get, which prevents doing distributed planning
for certain queries.

We wonder if it is possible to allow extensions to access the join info
under all circumstances, as it used to be? Basically, removing the
additional check:

diff --git a/src/backend/optimizer/path/joinpath.c
b/src/backend/optimizer/path/joinpath.c
index 03b3185984..080e76cbe9 100644
--- a/src/backend/optimizer/path/joinpath.c
+++ b/src/backend/optimizer/path/joinpath.c
@@ -349,8 +349,7 @@ add_paths_to_joinrel(PlannerInfo *root,
/*
* 6. Finally, give extensions a chance to manipulate the path list.
*/
- if (set_join_pathlist_hook &&
- consider_join_pushdown)
+ if (set_join_pathlist_hook)
set_join_pathlist_hook(root, joinrel, outerrel, innerrel,
jointype,
&extra);
}

Thanks,
Onder

[1]:
https://github.com/postgres/postgres/commit/b0e390e6d1d68b92e9983840941f8f6d9e083fe0
[2]: https://github.com/citusdata/citus/issues/7119

Etsuro Fujita <etsuro(dot)fujita(at)gmail(dot)com>, 15 Ağu 2023 Sal, 11:05 tarihinde
şunu yazdı:

> On Tue, Aug 8, 2023 at 6:30 PM Richard Guo <guofenglinux(at)gmail(dot)com> wrote:
> > On Tue, Aug 8, 2023 at 4:40 PM Etsuro Fujita <etsuro(dot)fujita(at)gmail(dot)com>
> wrote:
> >> I modified the code a bit further to use an if-test to avoid a useless
> >> function call, and added/tweaked comments and docs further. Attached
> >> is a new version of the patch. I am planning to commit this, if there
> >> are no objections.
>
> > +1 to the v4 patch. It looks good to me.
>
> Pushed after some copy-and-paste editing of comments/documents.
>
> Thanks!
>
> Best regards,
> Etsuro Fujita
>
>
>

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Heikki Linnakangas 2023-08-15 14:40:53 Re: BUG #17946: LC_MONETARY & DO LANGUAGE plperl - BUG
Previous Message John Naylor 2023-08-15 11:53:05 Re: [PoC] Improve dead tuple storage for lazy vacuum