Re: Function scan FDW pushdown

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.

> One more interesting example (included in the patch) is
>
> EXPLAIN (VERBOSE, COSTS OFF)
> WITH s AS MATERIALIZED (SELECT r1.* FROM remote_tbl r1
> JOIN LATERAL
> (SELECT r2.a FROM remote_tbl r2, f(r1.a) LIMIT 1) s
> ON true)
> SELECT * FROM s ORDER BY 1;
>
> We get the following plan:
>
> Sort
> Output: s.a, s.b
> Sort Key: s.a
> CTE s
> -> Nested Loop
> Output: r1.a, r1.b
> -> Foreign Scan on public.remote_tbl r1
> Output: r1.a, r1.b
> Remote SQL: SELECT a, b FROM public.base_tbl_fn
> -> Foreign Scan
> Output: NULL::integer
> Relations: (public.remote_tbl r2) INNER JOIN (Function
> f)
> Remote SQL: SELECT NULL FROM (public.base_tbl_fn r1
> INNER JOIN public.f($1::integer) f2(c1) ON (TRUE)) LIMIT 1::bigint
> -> CTE Scan on s
> Output: s.a, s.b
>
> Here you can see that we use parameter in function argument. Don't know
> if it's a real problem, but at least looks suspicious. In v3 patch used
> contain_param_walker() in is_nonrel_relinfo_ok() (which mutated to
> function_rte_pushdown_ok()) to avoid such plans.

AFAICS, there is no correctness problem.

* The bms_is_empty(rel->lateral_relids) check rejects a lateral
reference to a relation at the same query level.
* In the example, f(r1.a) references r1 from an *outer* level. When
the subquery is planned, r1.a becomes a PARAM_EXEC, and the function
rel's lateral_relids inside the subquery is empty -- so the check
passes and funcexpr retains a Param.
* foreign_expr_walker deliberately allows PARAM_EXEC
(deparse.c:486–505), and postgres_fdw's normal machinery (params_list
=> fdw_exprs) re-sends the param on each rescan.

So there's no need to bring back v3's contain_param_walker() — this is
actually a useful capability (a parameterized foreign scan). I only
add a comment in function_rte_pushdown_ok() noting that outer-level
Params are allowed intentionally and handled as an ordinary
parameterized foreign scan.

> One minor issue I've noticed is in function_rte_pushdown_ok():
> + if (rel->rtekind != RTE_FUNCTION)
> + return false;
> + rte = planner_rt_fetch(rel->relid, root);
> + if (rte->rtekind != RTE_FUNCTION)
> + return false;
>
> Is the second rtekind check necessary?

The second check is not necessary, changed to an assert.

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.

3. Changed redundant elog(ERROR) into Assert in get_functions_data()
and get_tupdesc_for_join_scan_tuples(). The return-type validity is
already guaranteed by function_rte_pushdown_ok(), so these are pure
cross-checks.

4. Style consistency in find_em_for_rel_target(): introduced a named
fpinfo local instead of passing rel->fdw_private inline, matching
find_em_for_rel().

5. Also added test for nullable-side whole-row Var, but with outer
rows that both match and miss the (foreign x function) inner join.

------
Regards,
Alexander Korotkov
Supabase