Re: Function scan FDW pushdown

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.
--
Best regards,
Alexander Pyhalov,
Postgres Professional