Hi Richard, On Mon, Jul 31, 2023 at 5:52 PM Richard Guo <guofengli...@gmail.com> wrote: > On Fri, Jul 28, 2023 at 4:56 PM Etsuro Fujita <etsuro.fuj...@gmail.com> wrote: >> 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. Ok, but maybe my explanation was not good, so let me explain. The reason why I modified the code as such is to make the handling of fdw_restrictinfo consistent with that of fdw_outerpath: we have the code to reparameterize fdw_outerpath, which should be NULL though, as we do not currently support parameterized foreign joins. 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. Thanks! Best regards, Etsuro Fujita
0002-Allow-join-pushdown-even-if-pseudoconstant-quals-v4.patch
Description: Binary data