On Fri, Jul 28, 2023 at 4:56 PM Etsuro Fujita <etsuro.fuj...@gmail.com> wrote:
> Cool! I pushed the first patch after polishing it a little bit, so > here is a rebased version of the second patch, in which I modified the > ForeignPath and CustomPath cases in reparameterize_path_by_child() to > reflect the new members fdw_restrictinfo and custom_restrictinfo, for > safety, and tweaked a comment a bit. Hmm, it seems that ForeignPath for a foreign join does not support parameterized paths for now, as in postgresGetForeignJoinPaths() we have this check: /* * This code does not work for joins with lateral references, since those * must have parameterized paths, which we don't generate yet. */ if (!bms_is_empty(joinrel->lateral_relids)) return; And in create_foreign_join_path() we just set the path.param_info to NULL. pathnode->path.param_info = NULL; /* XXX see above */ So I doubt that it's necessary to adjust fdw_restrictinfo in reparameterize_path_by_child, because it seems to me that fdw_restrictinfo must be empty there. Maybe we can add an Assert there as below: - ADJUST_CHILD_ATTRS(fpath->fdw_restrictinfo); + + /* + * Parameterized foreign joins are not supported. So this + * ForeignPath cannot be a foreign join and fdw_restrictinfo + * must be empty. + */ + Assert(fpath->fdw_restrictinfo == NIL); That being said, it's also no harm to handle fdw_restrictinfo in reparameterize_path_by_child as the patch does. So I'm OK we do that for safety. Thanks Richard