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

On Fri, Jul 21, 2023 at 8:51 PM Etsuro Fujita <etsuro.fujita@gmail.com>
wrote:

> I think we should choose the latter, so I modified your patch as
> mentioned, after re-creating it on top of my patch.  Attached is a new
> version (0002-Allow-join-pushdown-even-if-pseudoconstant-quals-v2.patch).
> I am attaching my patch as well
> (0001-Disable-join-pushdown-if-pseudoconstant-quals-v2.patch).
>
> Other changes made to your patch:
>
> * I renamed the new member of the ForeignPath struct to
> fdw_restrictinfo.  (And I named that of the CustomPath struct
> custom_restrictinfo.)


That's much better, and more consistent with other members in
ForeignPath/CustomPath.  Thanks!


> * In your patch, only for create_foreign_join_path(), the API was
> modified so that the caller provides the new member of ForeignPath,
> but I modified that for
> create_foreignscan_path()/create_foreign_upper_path() as well, for
> consistency.


LGTM.


> * In this bit I changed the last argument to NIL, which would be
> nitpicking, though.
>
> @@ -1038,7 +1038,7 @@ postgresGetForeignPaths(PlannerInfo *root,
>   add_path(baserel, (Path *) path);
>
>   /* Add paths with pathkeys */
> - add_paths_with_pathkeys_for_rel(root, baserel, NULL);
> + add_paths_with_pathkeys_for_rel(root, baserel, NULL, NULL);


Good catch!  This was my oversight.


> * I dropped this test case, because it would not be stable if the
> system clock was too slow.


Agreed.  And the test case from 0001 should be sufficient.

So the two patches both look good to me now.

Thanks
Richard