Re: Function scan FDW pushdown

From: Alexander Pyhalov <a(dot)pyhalov(at)postgrespro(dot)ru>
To: Alexander Korotkov <aekorotkov(at)gmail(dot)com>
Cc: solaimurugan vellaipandiyan <drsolaimurugan(dot)v(at)gmail(dot)com>, Álvaro Herrera <alvherre(at)kurilemu(dot)de>, g(dot)kashkin(at)postgrespro(dot)ru, Ashutosh Bapat <ashutosh(dot)bapat(dot)oss(at)gmail(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>
Subject: Re: Function scan FDW pushdown
Date: 2026-05-18 20:06:31
Message-ID: b02d45f5484fedb7e32acbed44abb8f6@postgrespro.ru
Views: Whole Thread | Raw Message | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Alexander Korotkov писал(а) 2026-05-18 13:34:
> Hi, Alexander!
>
> The revised patch is attached.
>
> On Tue, May 12, 2026 at 11:09 AM Alexander Pyhalov
> <a(dot)pyhalov(at)postgrespro(dot)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.

Hi. I am a bit confused about this comment (and code):

/*
* DirectModify on a foreign join: pass NIL/0 for
the function
* metadata. We don't currently push function
RTEs through the
* direct-modify path, so there are no whole-row
Vars pointing at
* function-RTE tuples to reconstruct.
*/
tupdesc = get_tupdesc_for_join_scan_tuples(node,
NIL, 0);

We evidently go through this code path when executing example

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

But don't need whole row var in returning list.... However, we still can
step on this issue.

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
CONTEXT: whole-row reference to foreign table "t"

--
Best regards,
Alexander Pyhalov,
Postgres Professional

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Álvaro Herrera 2026-05-18 20:32:45 Re: [PATCH] Prevent repeated deadlock-check signals in standby buffer pin waits
Previous Message Andrey Rachitskiy 2026-05-18 19:30:24 Re: [PATCH] ternary reloption type