Function scan FDW pushdown

First seen: 2026-03-18 12:08:49+00:00 · Messages: 14 · Participants: 3

Latest Update

2026-05-22 · claude-opus-4-6

New in this round: Pyhalov identifies architectural weakness in Korotkov's "no fpinfo on function rel" design — surfaces three concrete problems

A single dense message from Pyhalov that challenges a core design decision in v5/v6 and provides a patch attempting to fix one of the issues. This is not a bug report against specific queries (as in the prior round) but a structural critique of where metadata lives.

1. The "no fdw_private on function RelOptInfo" design creates downstream problems

Korotkov's v4/v5 deliberately avoided attaching PgFdwRelationInfo to the function's RelOptInfo, putting it instead on the joinrel. Pyhalov now argues this was a mistake, identifying three concrete consequences:

Problem A: baserestrictinfo classification fails. When a function RTE has restriction clauses (e.g., WHERE g.i > 5 on a generate_series() alias), classifyConditions() needs to determine which restrictions are shippable to the remote. But foreign_expr_walker() — the function that validates expression shippability — needs access to fdw_private (specifically PgFdwRelationInfo) to resolve column references against the function RTE. With no fpinfo on the function rel, the walker has no context to validate expressions referencing function output columns.

Pyhalov's attached patch addresses this by passing fpinfo down into foreign_expr_walker() — but he flags the awkwardness: should fpinfo be temporarily attached to the function rel before classifyConditions() and then removed? This is a sign the architecture is fighting itself.

Problem B: Subquery-generating join optimizations. When join uniqueness optimizations (INNER UNIQUE/OUTER UNIQUE) trigger subquery creation, the planner calls get_relation_column_alias_ids(), which needs fpinfo for the function RTE. Without it, these code paths would fail or need special-casing.

Problem C: Param-containing function arguments are sneaking through. Pyhalov demonstrates with a LATERAL + LIMIT subquery example:

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;

The plan shows f($1::integer) in the remote SQL — a Param node is being sent to the remote, which was explicitly forbidden in the v2/v3 restriction set. Korotkov's v5 dropped contain_param_walker() from function_rte_pushdown_ok(), and this case demonstrates that the old check was load-bearing. The remote execution of f($1) is "suspicious" (Pyhalov's word) — it means the remote side receives a parameterized function call that depends on a value from a different scan level, which may or may not resolve correctly depending on how the remote cursor/prepared statement handles it.

2. Minor code observation: redundant rtekind check

Pyhalov notes that function_rte_pushdown_ok() checks rel->rtekind != RTE_FUNCTION and then immediately fetches the RTE and checks rte->rtekind != RTE_FUNCTION again. Since rel->rtekind is copied from the RTE during build_simple_rel(), the second check is redundant. Minor, but indicative of incomplete cleanup in the rapid iteration.

Assessment

This message reveals that the "fpinfo lives only on the joinrel" design, while theoretically clean for the multi-server case, creates practical difficulties whenever other planner/deparser infrastructure needs metadata about the function RTE in isolation (not as part of a join). The three problems Pyhalov raises all stem from the same root: code paths that operate on individual base rels (restriction classification, alias resolution, expression walking) need per-rel metadata that doesn't exist.

The likely resolution is a hybrid: a lightweight fpinfo is attached to the function RelOptInfo (sufficient for expression walking and alias resolution), while the full join-costing fpinfo remains on the joinrel. This would be a partial revert toward Pyhalov's original v2/v3 architecture with Korotkov's multi-absorption improvements layered on top.

The Param issue is the more urgent concern — it's a correctness hole, not just an architectural inconvenience. The contain_param_walker() check needs to be restored.

History (2 prior analyses)
2026-05-20 · claude-opus-4-6

New in this round: Korotkov posts v5 addressing all five Pyhalov bugs, then two rapid follow-up fixups for whole-row Var edge cases in DirectModify/outer-join paths

This round shows the patch entering a tight collaborative iteration cycle between Korotkov and Pyhalov, with three patch revisions in two days. The bugs from the prior round are resolved, but new edge cases in whole-row Var handling continue to surface.

1. Korotkov's v5 comprehensively addresses Pyhalov's five-bug report

The revision fixes all issues raised in the prior round:

Composite-return restriction restored (Bug 2): function_rte_pushdown_ok() now calls get_expr_result_type() and rejects anything that isn't TYPEFUNC_SCALAR (plus explicit rejection of RECORDOID/VOIDOID). This is the "IMMUTABLE plus scalar-return-only" layering the prior analysis predicted.

Whole-row Var for function RTEs (Bug 1): deparseColumnRef() now handles varattno == 0 for RTE_FUNCTION by emitting ROW(f<rti>.c1, ..., f<rti>.cN) constructed from rte->eref->colnames. This is a synthetic row-constructor — the remote side never sees a true whole-row reference to a function alias, just an explicit ROW(...).

RTE-type guard in executor (Bug 3): The scanrelid == 0 branch in postgresBeginForeignScan() now iterates fs_base_relids until it finds an RTE_RELATION, with an elog(ERROR) guard for the impossible case where none is found.

Tuple descriptor reconstruction (Bug 4): get_tupdesc_for_join_scan_tuples() now has an RTE_FUNCTION branch that rebuilds the TupleDesc from metadata saved in fdw_private. The metadata format is: FdwScanPrivateFunctions (a list of per-function (funcid, funcrettype, funccollation) tuples indexed by RTI-offset) plus FdwScanPrivateMinRTIndex. This is essentially Pyhalov's v2/v3 approach restored on top of Korotkov's structural changes.

Multi-function ROWS FROM support (Bug 5 / design question): function_rte_pushdown_ok() now loops over all entries in rte->functions and applies shippability/type checks to each. deparseFunctionRangeTblRef() emits ROWS FROM (f1(), f2(), ...) AS f<rti>(c1, c2, ...) with column names covering the concatenation of all functions' columns. This restores the HammerDB UNNEST(arr1, arr2, ...) use case.

2. New architectural detail: fpinfo for function lives on the joinrel, not the function RelOptInfo

Korotkov explains a design decision not previously visible: the function-side fpinfo is stored as outer_func_fpinfo/inner_func_fpinfo fields on the joinrel's PgFdwRelationInfo, rather than on the function RelOptInfo->fdw_private. The reason: the same RTE_FUNCTION rel can be paired independently with foreign rels from different servers. Putting fpinfo on the function rel would force a single-server assumption; putting it on the joinrel keeps it per-pairing. This is clean and consistent with how the stub-fpinfo from the prior round actually materializes.

3. LATERAL explicitly rejected; outer joins excluded

Two scope-limiting decisions are now codified:

  • function_rte_pushdown_ok() returns false when rel->lateral_relids is non-empty. This confirms what the prior analysis suspected: LATERAL function scans are out of scope for v1.
  • Only JOIN_INNER sets inner_is_function/outer_is_function; all other join types (LEFT, RIGHT, FULL, SEMI) fall through to the existing !fpinfo_o || !fpinfo_i rejection. This is conservative but correct — outer-join semantics with a function RTE that might produce no rows remotely vs. locally are tricky and best deferred.

4. Pyhalov finds a remaining edge case: whole-row Var on nullable outer side in DirectModify

Pyhalov points out that the DirectModify path passes NIL/0 for function metadata to get_tupdesc_for_join_scan_tuples(), which works as long as no whole-row Var references a function RTE. But with RETURNING t (where t is the function alias), a whole-row Var is needed. He demonstrates:

UPDATE remote_tbl r SET b=5
FROM UNNEST(array[box '((2,3),(-2,-3))'], array[int '1']) AS t (bx, i)
WHERE r.a = area(t.bx) RETURNING a, b, t;

→ ERROR: input of anonymous composite types is not implemented

Additionally, when the function RTE is on a nullable (outer) join side, the deparsed ROW(...) produces (NULL, NULL) instead of a proper SQL NULL. The fix requires applying the same COALESCE/null-detection logic used for nullable table column references.

5. Korotkov accepts the nullable-side fix, adds Assert for impossible NULL rtfuncdata

Korotkov's response (v6 or v5.1):

  • Accepts Pyhalov's nullable-outer-side fix for whole-row Var deparsing.
  • Adds Pyhalov's RETURNING t query as a regression test.
  • For the question "can rtfuncdata be NULL when we hit an RTE_FUNCTION in get_tupdesc_for_join_scan_tuples()?" — Korotkov says he doesn't see how it's possible and adds an Assert(rtfuncdata != NIL) / Assert(funcdata != NULL) rather than a runtime fallback. This is a defensible choice: if it ever fires, it indicates a planning-stage bug (metadata not saved), not a user-facing error.

Summary of patch state

The patch is now in rapid bug-fix convergence. The major architectural questions are settled:

  • IMMUTABLE + scalar-return + no-Param + no-LATERAL + INNER-only = restriction set
  • fpinfo on joinrel, metadata in fdw_private, multi-function ROWS FROM supported
  • Planner-side change minimal (one branch in set_foreign_rel_properties())

Remaining open items are increasingly narrow edge cases rather than design disagreements. The collaborative dynamic between Korotkov (committer, driving structure) and Pyhalov (domain expert, stress-testing edges) is productive and converging.

2026-05-14 · claude-opus-4-6

New in this round: Pyhalov stress-tests Korotkov's v4, finds five concrete bugs/regressions — reveals that the IMMUTABLE-only tightening came at the cost of dropping safety checks that were load-bearing

This is the most technically dense round yet. Pyhalov (the original patch author) has systematically tested Korotkov's revision and surfaced five distinct issues, ranging from outright crashes to silent semantic incorrectness. Solaimurugan also returns with a confirmation that pushdown still doesn't trigger without use_remote_estimate, which Korotkov explains is a pre-existing cost-model limitation, not a patch bug.

1. Solaimurugan confirms: pushdown requires use_remote_estimate

Solaimurugan retested v4 and still could not observe pushdown with default cost settings, even after inserting 1000 rows and running ANALYZE. Korotkov's reply demonstrates that with the default local cost model, the local Hash Join always wins for this table size. Only after ALTER FOREIGN TABLE ft1 OPTIONS (ADD use_remote_estimate 'true') does the remote join appear. Korotkov frames this as a general postgres_fdw cost-model inaccuracy, not a patch-specific problem. This is accurate — but it means the feature is effectively gated behind use_remote_estimate for typical workloads, which is a significant usability limitation worth documenting.

The key insight from Korotkov's cost walkthrough: estimate_path_cost_size() applies the join operator to the full cross-product estimate, which inflates the remote-join cost. Only when the remote server provides its own EXPLAIN estimate (via use_remote_estimate) does the remote path win, because the remote optimizer knows the join will filter aggressively.

2. Pyhalov's five-bug report on Korotkov's v4

This is the heart of the round. Pyhalov tested Korotkov's revision against edge cases the original patch handled and found that several safety mechanisms were dropped:

Bug 1: deparseColumnRef() crashes on whole-row Vars for function RTEs

UPDATE remote_tbl r SET b=5 FROM UNNEST(array[box '((2,3),(-2,-3))']) AS t (bx)
WHERE r.a = area(t.bx)

Fails with an assert that varattno > 0. When a function RTE is "locked" (e.g., for UPDATE target), a whole-row Var (varattno = 0) is generated, and deparseColumnRef() has no code path to handle whole-row references to a function RTE. The old patch apparently handled this; the new revision dropped the handling.

Bug 2: f_ret_record() returning RECORD now crashes instead of being rejected

Korotkov removed the composite-return-type check from function_rte_pushdown_ok(), relying on IMMUTABLE alone as the gate. But the deparser still cannot handle RECORD-returning functions with column definition lists — the remote side receives a bare function call without the AS (a int, b int) decoration, producing:

ERROR: a column definition list is required for functions returning "record"

This confirms what the prior analysis suspected: the composite-return and Param-argument restrictions are orthogonal to IMMUTABLE and were load-bearing safety checks, not redundant ones. Dropping them was a regression.

Bug 3: postgresBeginForeignScan() hits a function RTE and fails with "cache lookup failed for foreign table 0"

SELECT * FROM unnest(array[2,3,4]) n, remote_tbl r WHERE r.a = n;

When iterating RTEs in the scan's relid set, the executor encounters the function RTE and tries to look up its foreign table metadata — which doesn't exist. The older patch version explicitly skipped non-RTE_RELATION entries; Korotkov's revision lost that guard.

Bug 4: get_tupdesc_for_join_scan_tuples() doesn't handle function RTEs, causing type-resolution failures

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;

The join tuple descriptor construction doesn't know how to derive column types from a function RTE. Currently this manifests as "column f2.c0 does not exist" (an earlier deparse failure masks it), but even if that's fixed, it would hit "input of anonymous composite types is not implemented." This was a significant chunk of complexity in the original patch that Korotkov's streamlining removed.

Bug 5 (design question): function_rte_pushdown_ok() is now too permissive

Pyhalov raises two specific concerns:

  • Can the shippability check skip Vars from outside joinrel->relids? (i.e., could a function argument reference a relation not part of the pushed-down join, leading to an unresolvable remote reference)
  • Can functions with Param arguments actually be safely shipped? (The old patch said no; the new version apparently doesn't check.)

These are open questions, not demonstrated bugs, but they suggest the tightening-to-IMMUTABLE was treated as a replacement for the old restriction set rather than an addition to it.

3. Pyhalov asks why list_length(rte->functions) is restricted to 1

The ROWS FROM(f1(...), f2(...)) syntax produces an RTE with multiple entries in rte->functions. The old patch apparently supported this because UNNEST() with multiple arrays — used in the HammerDB tproc-c benchmark — generates exactly this pattern. Restricting to a single function entry eliminates an important real-world use case.

4. Pyhalov flags broader extensibility: VALUES RTEs

He notes that the architectural approach taken here (modifying set_foreign_rel_properties() in core to recognize non-relation RTEs) could also support pushing down VALUES lists, but the current patch is specific to RTE_FUNCTION. This is forward-looking commentary, not a blocking concern.

Assessment

Korotkov's v4 streamlined the patch but over-simplified it, removing safety checks that were necessary for correctness. Pyhalov's bug report is essentially a revert request for three specific mechanisms:

  1. The composite-return-type restriction in function_rte_pushdown_ok()
  2. The RTE-type guard in postgresBeginForeignScan()
  3. The function-RTE handling in get_tupdesc_for_join_scan_tuples()

The IMMUTABLE restriction itself is a good addition (as argued in the prior analysis), but it needs to be layered on top of the existing restrictions, not substituted for them. The next revision will likely be a merge of Korotkov's structural improvements (stub fpinfo, multi-absorption-path enumeration) with Pyhalov's original safety machinery.