Thread

Re: Function scan FDW pushdown

Alexander Korotkov <aekorotkov@gmail.com> — 2026-05-18T10:34:21Z

Hi, Alexander!

The revised patch is attached.

On Tue, May 12, 2026 at 11:09 AM Alexander Pyhalov
<a.pyhalov@postgrespro.ru> wrote:
> 1) deparseColumnRef() doesn't account for whole row vars.
> In queries like
>
> UPDATE remote_tbl r SET b=5 FROM UNNEST(array[box '((2,3),(-2,-3))']) AS
> t (bx) WHERE r.a = area(t.bx)
>
> it fails with assert that varattno should be > 0. When we lock
> non-relation RTE, we select whole row var, and we have to deparse it for
> function RTE.
>
> You've removed check for function return type. This seems to be
> dangerous. Old example
>
> CREATE OR REPLACE FUNCTION f_ret_record() RETURNS record AS $$ SELECT
> (1,2)::record $$ language SQL IMMUTABLE;
> ALTER EXTENSION postgres_fdw ADD function f_ret_record();
> EXPLAIN (VERBOSE, COSTS OFF)
> SELECT s FROM remote_tbl rt, f_ret_record() AS s(a int, b int)
> WHERE s.a = rt.a;
>
> fails with
>
> ERROR:  a column definition list is required for functions returning
> "record"

function_rte_pushdown_ok() now calls get_expr_result_type() and
rejects anything that isn't TYPEFUNC_SCALAR (also RECORDOID/VOIDOID),
so f_ret_record() no longer reaches the remote side.
deparseColumnRef() now handles varattno == 0 for RTE_FUNCTION and
emits ROW(f<rti>.c1, ..., f<rti>.c<N>) from rte->eref->colnames.

> 2) postgresBeginForeignScan() can step on function RTE, and doesn't know
> what to do with it:
> SELECT * FROM unnest(array[2,3,4]) n, remote_tbl r WHERE r.a = n;
> ERROR:  cache lookup failed for foreign table 0
>
> So, we need to look for the first RTE_RELATION, as in older patch
> version.

The scanrelid == 0 branch in postgresBeginForeignScan() now scans
fs_base_relids until it finds an RTE_RELATION.  An explicit
elog(ERROR) guards the (theoretically impossible) case where no
foreign RTE is found.

> 3) A lot of complexity in the old patch version was in making it
> possible to find out RTE_FUNCTION attribute types after planing, as it's
> necessary to correctly handle joins. In this version
> get_tupdesc_for_join_scan_tuples() doesn't handle function RTEs.  This
> means, when we try to find out type for attribute types for joins, we'll
> get errors. This can be seen in queries like
>
> UPDATE remote_tbl r SET b=CASE WHEN random()>=0 THEN 5 ELSE 0 END FROM
> UNNEST(array[box '((2,3),(-2,-3))']) AS t (bx) WHERE r.a = area(t.bx)
>   RETURNING a,b;
>
> Now it fails on earlier stages (with "column f2.c0 does not exist"), but
> if we fix it, we'll get something like
> "ERROR:  input of anonymous composite types is not implemented"
>
> Overall, function_rte_pushdown_ok() now allows more strange
> constructions. Could it skip Vars from outside of joinrel->relids? Can
> we safely ship function with parameters in arguments? I'm not sure.

Restored the per-function metadata you had in v2/v3.
FdwScanPrivateFunctions (list of (funcid, funcrettype, funccollation)
indexed by RTI-offset) and FdwScanPrivateMinRTIndex are now saved in
fdw_private by postgresGetForeignPlan().
get_tupdesc_for_join_scan_tuples() now has an RTE_FUNCTION branch that
rebuilds the tuple descriptor from this metadata, exactly as in your
patch.

> 4) Why do we restrict list_length(rte->functions) to 1? The main reason
> for supporting several rte->functions was to allow pushdown of UNNEST()
> with several arrays, which is used by HammerDB tproc-c test.

function_rte_pushdown_ok() now loops over rte->functions and applies
the same checks to every member.  deparseFunctionRangeTblRef() emits
ROWS FROM (f1(), f2(), ...) AS f<rti>(c1, c2, ...) with the
column-name list covering the concatenation of every function's
columns, in the order they appear.

> 5) We can support pushing down joins of foreign tables and another RTE
> types, for example, VALUES. But with new specific way of handling
> RTE_FUNCTIONS in core, this requires both changes to
> set_foreign_rel_properties() and postgres_fdw. Not sure, if this is a
> big problem, but at least is worth mentioning .

I've kept the planner-side change minimal:
set_foreign_rel_properties() propagates fdwroutine onto a joinrel
pairing a foreign rel with an RTE_FUNCTION rel, so the standard
GetForeignJoinPaths callback gets invoked.  No new FDW callback was
needed.  This is also the reason the planner-side fpinfo of the
function side lives on the joinrel
(outer_func_fpinfo/inner_func_fpinfo fields added to
PgFdwRelationInfo) rather than on the function
RelOptInfo->fdw_private: the same RTE_FUNCTION rel can be paired
independently with foreign rels from several servers.  Extending the
same scheme to RTE_VALUES / RTE_CTE is, I agree, worth doing late. It
would be a one branch addition to set_foreign_rel_properties() plus an
FDW-side shippability check analogous to function_rte_pushdown_ok().

A few extra changes to the patch:
- LATERAL function RTEs are explicitly rejected in
function_rte_pushdown_ok() (returns false when rel->lateral_relids is
non-empty).
- Outer joins (LEFT/RIGHT/FULL) and SEMI joins fall through to the
existing !fpinfo_o || !fpinfo_i rejection, since
inner_is_function/outer_is_function are only set for JOIN_INNER.
- Regression coverage in contrib/postgres_fdw/sql/postgres_fdw.sql now
reuses your examples.

------
Regards,
Alexander Korotkov
Supabase