Re: Function scan FDW pushdown

public inbox for [email protected]  
help / color / mirror / Atom feed

From: Alexander Pyhalov <[email protected]>
To: Alexander Korotkov <[email protected]>
Cc: solaimurugan vellaipandiyan <[email protected]>
Cc: Álvaro Herrera <[email protected]>
Cc: [email protected]
Cc: Ashutosh Bapat <[email protected]>
Cc: PostgreSQL Hackers <[email protected]>
Subject: Re: Function scan FDW pushdown
Date: Mon, 18 May 2026 23:06:31 +0300
Message-ID: <[email protected]> (raw)
In-Reply-To: <CAPpHfduVg4-6U=oDyBLGV53nU6bVfF+3yqVo9s09TpRSjgMDWw@mail.gmail.com>
References: <[email protected]>
	<[email protected]>
	<[email protected]>
	<CAHEL7KQU_=iKQU3dGMV+6vOBWkh=i5g7speEU=VVziRu=3v1Tw@mail.gmail.com>
	<[email protected]>
	<CAPpHfdvz=ahOKEnu4OZ_z3JjQQVddgsNti4FX8SmAKCxmh80BQ@mail.gmail.com>
	<[email protected]>
	<CAPpHfduVg4-6U=oDyBLGV53nU6bVfF+3yqVo9s09TpRSjgMDWw@mail.gmail.com>

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
> <[email protected]> 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

reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: [email protected]
  Cc: [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected]
  Subject: Re: Function scan FDW pushdown
  In-Reply-To: <[email protected]>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox