On Fri, Jul 21, 2023 at 8:51 PM Etsuro Fujita <etsuro.fuj...@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
