Re: Function scan FDW pushdown

On Tue, Jul 7, 2026 at 12:25 PM Alexander Pyhalov
<a(dot)pyhalov(at)postgrespro(dot)ru> wrote:
>
> Alexander Korotkov писал(а) 2026-07-06 17:11:
> > Hi, Alexander!
> >
> > On Wed, May 20, 2026 at 1:17 PM Alexander Pyhalov
> > <a(dot)pyhalov(at)postgrespro(dot)ru> wrote:
> >> I've found another issue. The fact that in the new versions of the
> >> patch
> >> RTE RelOptInfo misses fdw_private seems to be unfortunate. For
> >> example,
> >> in the last version we haven't thought about classifying
> >> baserestrictinfo. And if we do, we should pass fdw_private down to
> >> foreign_expr_walker. Perhaps, we could attach it to RTE_FUNCTION rel
> >> prior to calling classifyConditions(), but should we later set it back
> >> to NULL? Another problem comes if we try to handle joins, which can
> >> crearte subqueries (like INNER/OUTER UNIQUE). In this case we should
> >> somehow cook fpinfo for get_relation_column_alias_ids(). Attaching
> >> patch
> >> which tries to handle baserestrictinfos by passing fpinfo down to
> >> foreign_expr_walker().
> >
> > I think passing fpinfo down to foreign_expr_walker() is the right
> > approach, thank you. Attaching it to RTE_FUNCTION rel, and reseting
> > it back to NULL would create excessive state and potential source of
> > bugs. The only advantage is saving existing function signatures, but
> > it doesn't worth it.
> >
> > Also, I don't think we need to care bout
> > get_relation_column_alias_ids(). As we currently pushdown function
> > only for inner joins, make_outerrel_subquery/make_innerrel_subquery
> > are always false, lower_subquery_rels also wouldn't contain
> > RTE_FUNCTION relid.
> > Therefore:
> >
> > * In deparseFromExprForRel() for functional rel deparseRangeTblRef()
> > is always called with make_subquery=false.
> > * is_subquery_var() returns false.
>
> Well, at least current patch version classifies conditions correctly.
>
> > Other changes I made:
> >
> > 1. In foreign_join_ok(), the function side's baserestrictinfo was
> > classified against the joinrel fpinfo, whose
> > server/shippable_extensions aren't set until merge_fdw_options() runs
> > later — so extension-shippable quals would be misclassified as local
> > and silently kill the pushdown. Now it: (a) copies
> > shippable_extensions from the foreign side onto the stub, and (b)
> > classifies against the stub (fpinfo_i/fpinfo_o, which already has
> > server). Built-in quals like n > 3 were unaffected; this fixes the
> > extension case. I've added the test case for this.
> >
> > 2. Dropped unused funcid. The per-RTE metadata was a 3-element list
> > (funcid, funcrettype, funccollation) but funcid was never read.
> > Reduced to list_make2(funcrettype, funccollation); updated the
> > consumer (linitial/lsecond) and both descriptive comments.
>
> With funcid we could print actual function name in explain.
> Now we print, for example,
>
> Relations: ((public.distr1_part_3 d1_3) INNER JOIN (public.distr2_part_3
> d2_3)) INNER JOIN (Function n)
>
> If funcid is preserved, we could output actual function name instead of
> alias. This is more similar to
> how we show relation names. Attaching updated patch, which preserves
> function names in explain.
>
> After looking at it once more, I'm not convinced when we can get
> not-FuncExpr in rtfunc->funcexpr.
> So added more checks to function_rte_pushdown_ok() to avoid dealing with
> such case.

I agree on your changes. I've put some additional.

1) Extracted initialization of stub fpinfo for function RTE into
init_func_stub_fpinfo(). The stub now inherits fdw_startup_cost,
fdw_tuple_cost, use_remote_estimate, fetch_size and async_capable from
the foreign side, in addition to server and shippable_extensions.
Without this, merge_fdw_options() and the cost estimator would pick up
zeroed values of fdw_startup_cost and fdw_tuple_cost when the function
RTEis on the outer side of the join, so the pushdown path appears to
be artificially cheap.
2) Added a comment on the whole-row Var CASE WHEN wrapper in
deparseColumnRef() explaining that it is a no-op under the current
INNER-only policy but kept to stay correct for future outer-join and
null-supply scenarios. Please, check if you agree with this.
3) Documented the new pushdown case in postgres-fdw.sgml, listing the
conditions under which a foreign-table x set-returning-function INNER
join is absorbed into the remote query.
4) Added a regression test verifying that a STABLE (not IMMUTABLE)
set- returning function is rejected for pushdown, alongside the
existing VOLATILE-rejection test.

------
Regards,
Alexander Korotkov
Supabase