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

Reply via email to