MIME-Version: 1.0
Date: Mon, 18 May 2026 23:06:31 +0300
From: Alexander Pyhalov <a.pyhalov@postgrespro.ru>
To: Alexander Korotkov <aekorotkov@gmail.com>
Cc: solaimurugan vellaipandiyan <drsolaimurugan.v@gmail.com>,
 =?UTF-8?Q?=C3=81lvaro_Herrera?= <alvherre@kurilemu.de>,
 g.kashkin@postgrespro.ru, Ashutosh Bapat <ashutosh.bapat.oss@gmail.com>,
 PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
Subject: Re: Function scan FDW pushdown
In-Reply-To: 
 <CAPpHfduVg4-6U=oDyBLGV53nU6bVfF+3yqVo9s09TpRSjgMDWw@mail.gmail.com>
References: <202508051955.sao7ucalcce7@alvherre.pgsql>
 <e3af56f9b2f1bd9fdf12aff6ca25b18d@postgrespro.ru>
 <c42480257bc3350c20749f6c0362826c@postgrespro.ru>
 <CAHEL7KQU_=iKQU3dGMV+6vOBWkh=i5g7speEU=VVziRu=3v1Tw@mail.gmail.com>
 <b91abe323f12f4210a53feb1ead3ac0d@postgrespro.ru>
 <CAPpHfdvz=ahOKEnu4OZ_z3JjQQVddgsNti4FX8SmAKCxmh80BQ@mail.gmail.com>
 <7e2abeb3ea9bc7ca024f4e457dce33f5@postgrespro.ru>
 <CAPpHfduVg4-6U=oDyBLGV53nU6bVfF+3yqVo9s09TpRSjgMDWw@mail.gmail.com>
Message-ID: <b02d45f5484fedb7e32acbed44abb8f6@postgrespro.ru>
Content-Type: text/plain; charset=UTF-8;
 format=flowed
Content-Transfer-Encoding: 8bit
Archived-At: 
 <https://www.postgresql.org/message-id/b02d45f5484fedb7e32acbed44abb8f6%40postgrespro.ru>
Precedence: bulk

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.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.

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