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

Hi,

On Tue, Aug 15, 2023 at 11:02 PM Önder Kalacı <onderkalaci@gmail.com> wrote:
> 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);

Maybe we could do so by leaving to extensions the decision whether
they replace joins with pseudoconstant clauses, but I am not sure that
that is a good idea, because that would require the authors to modify
and recompile their extensions to fix the issue...  So I fixed the
core side.

I am not familiar with the Citus extension, but such pseudoconstant
clauses are handled within the Citus extension?

Thanks for the report!

Best regards,
Etsuro Fujita