Horton

Thread

Commits

  1. Fix a typo in the deparseArrayCoerceExpr() header comment

  2. Support for deparsing of ArrayCoerceExpr node in contrib/postgres_fdw

  1. postgres_fdw could deparse ArrayCoerceExpr

    Alexander Pyhalov <a.pyhalov@postgrespro.ru> — 2024-11-28T14:57:32Z

    Hi.

Recently, we were surprised by the following behavior - prepared 
statement, selecting data from foreign table with varchar(N) field 
couldn't push down "field = ANY($1)" expression, when switched to 
generic plan. This looked like shown in the attached patch. Reproducer 
is simple:

create extension postgres_fdw;
create server local foreign data wrapper postgres_fdw;
create user MAPPING FOR CURRENT_USER SERVER local;
create table test (c varchar(255));
create foreign table ftest (c varchar(255)) server local options 
(table_name 'test');

set plan_cache_mode to force_generic_plan ; -- just for demonstration, 
can happen with defautl plan_cache_mode, if planner decides that generic 
plan is preferable

prepare s(varchar[]) as select * from ftest where c = any ($1);

explain verbose execute s('{test}');
                               QUERY PLAN
----------------------------------------------------------------------
  Foreign Scan on public.ftest  (cost=100.00..143.43 rows=7 width=516)
    Output: c
    Filter: ((ftest.c)::text = ANY (($1)::text[]))
    Remote SQL: SELECT c FROM public.test

The issue is that we need to translate input array type from varchar[] 
to text[].

Attaching patch to allow postgres_fdw to deparse such conversion.


-- 
Best regards,
Alexander Pyhalov,
Postgres Professional

  2. Re: postgres_fdw could deparse ArrayCoerceExpr

    Maxim Orlov <orlovmg@gmail.com> — 2025-01-24T15:09:14Z

    Look like an overlook for me. Apparently no one has encountered this use
case before.

Patch seems good to me with no visible defects. Deparse support was also
added. As well as a
test case. But do we really need copy/paste code for a T_ArrayCoerceExpr
case? To be more specific,
can we "reuse" T_RelabelType case, as it made for T_OpExpr and
T_DistinctExpr?

-- 
Best regards,
Maxim Orlov.

  3. Re: postgres_fdw could deparse ArrayCoerceExpr

    Alexander Pyhalov <a.pyhalov@postgrespro.ru> — 2025-01-27T06:46:35Z

    Maxim Orlov писал(а) 2025-01-24 18:09:
> Look like an overlook for me. Apparently no one has encountered this
> use case before.
> 
> Patch seems good to me with no visible defects. Deparse support was
> also added. As well as a
> test case. But do we really need copy/paste code for a
> T_ArrayCoerceExpr case? To be more specific,
> can we "reuse" T_RelabelType case, as it made for T_OpExpr and
> T_DistinctExpr?
> 
> --
> 

Unfortunately, it's not so simple. We can't just ship type casts to 
remote server if we are not sure that local and remote types match. For 
example,

CREATE TYPE enum_of_int_like AS enum('1', '2', '3', '4');
CREATE TABLE conversions(id int, d enum_of_int_like);
CREATE FOREIGN TABLE ft_conversions (id int, d char(1))
SERVER loopback options (table_name 'conversions');
INSERT INTO ft_conversions VALUES (1, '1'), (2, '2'), (3, '3'), (4, 
'4');

Patched version gives error:

-- Test array type conversion pushdown
SET plan_cache_mode = force_generic_plan;
PREPARE s(varchar[]) AS SELECT count(*) FROM ft_conversions WHERE d = 
ANY ($1);
EXPLAIN (VERBOSE, COSTS OFF)
EXECUTE s(ARRAY['1','2']);
                                             QUERY PLAN
---------------------------------------------------------------------------------------------------
  Foreign Scan
    Output: (count(*))
    Relations: Aggregate on (public.ft_conversions)
    Remote SQL: SELECT count(*) FROM public.conversions WHERE ((d = ANY 
($1::character varying[])))
(4 rows)

EXECUTE s(ARRAY['1','2']);
ERROR:  operator does not exist: public.enum_of_int_like = character 
varying
HINT:  No operator matches the given name and argument types. You might 
need to add explicit type casts.
CONTEXT:  remote SQL command: SELECT count(*) FROM public.conversions 
WHERE ((d = ANY ($1::character varying[])))

Original one does successful local filtering:

PREPARE s(varchar[]) AS SELECT count(*) FROM ft_conversions WHERE d = 
ANY ($1);
EXPLAIN (VERBOSE, COSTS OFF)
EXECUTE s(ARRAY['1','2']);
                         QUERY PLAN
-----------------------------------------------------------
  Aggregate
    Output: count(*)
    ->  Foreign Scan on public.ft_conversions
          Output: id, d
          Filter: (ft_conversions.d = ANY (($1)::bpchar[]))
          Remote SQL: SELECT d FROM public.conversions
(6 rows)

EXECUTE s(ARRAY['1','2']);
  count
-------
      2

-- 
Best regards,
Alexander Pyhalov,
Postgres Professional

  4. Re: postgres_fdw could deparse ArrayCoerceExpr

    Maxim Orlov <orlovmg@gmail.com> — 2025-01-27T14:15:28Z

    On Mon, 27 Jan 2025 at 09:46, Alexander Pyhalov <a.pyhalov@postgrespro.ru>
wrote:

> Unfortunately, it's not so simple. We can't just ship type casts to
> remote server if we are not sure that local and remote types match. For
> example,
>

Yeah, my fault. I've overlooked an "elemexpr" member in "ArrayCoerceExpr"
and erroneously
consider them to have the same structure. Maybe some refactoring may be
done here, but,
obviously, this is not a goal of this patch

-- 
Best regards,
Maxim Orlov.

  5. Re: postgres_fdw could deparse ArrayCoerceExpr

    Maxim Orlov <orlovmg@gmail.com> — 2025-01-27T15:07:53Z

    So, patch looks good to me. Implements described functionality. Test case
also provided.
I think it's ready to be viewed by a committer.

-- 
Best regards,
Maxim Orlov.

  6. Re: postgres_fdw could deparse ArrayCoerceExpr

    Maxim Orlov <orlovmg@gmail.com> — 2025-01-29T09:58:55Z

    One important note here. This patch will change cast behaviour in case of
local and foreign types are mismatched.
The problem is if we cannot convert types locally, this does not mean that
it is also true for a foreign wrapped data.
In any case, it's up to the committer to decide whether this change is
needed or not.

-- 
Best regards,
Maxim Orlov.

  7. Re: postgres_fdw could deparse ArrayCoerceExpr

    Alexander Korotkov <aekorotkov@gmail.com> — 2025-06-04T11:29:04Z

    Hi Maxim,

On Wed, Jan 29, 2025 at 11:59 AM Maxim Orlov <orlovmg@gmail.com> wrote:
>
> One important note here. This patch will change cast behaviour in case of local and foreign types are mismatched.
> The problem is if we cannot convert types locally, this does not mean that it is also true for a foreign wrapped data.
> In any case, it's up to the committer to decide whether this change is needed or not.

I have two question regarding this aspect.
1) Is it the same with regular type conversion?
2) Can we fallback to remote type conversion in local type conversion fails?

------
Regards,
Alexander Korotkov
Supabase

  8. Re: postgres_fdw could deparse ArrayCoerceExpr

    Alexander Pyhalov <a.pyhalov@postgrespro.ru> — 2025-06-04T15:15:11Z

    Hi.

Alexander Korotkov писал(а) 2025-06-04 14:29:
> On Wed, Jan 29, 2025 at 11:59 AM Maxim Orlov <orlovmg@gmail.com> wrote:
>> 
>> One important note here. This patch will change cast behaviour in case 
>> of local and foreign types are mismatched.
>> The problem is if we cannot convert types locally, this does not mean 
>> that it is also true for a foreign wrapped data.
>> In any case, it's up to the committer to decide whether this change is 
>> needed or not.
> 
> I have two question regarding this aspect.
> 1) Is it the same with regular type conversion?

Yes, it's the same.

CREATE TYPE enum_of_int_like AS enum('1', '2', '3', '4');
CREATE TABLE conversions(id int, d enum_of_int_like);
CREATE FOREIGN TABLE ft_conversions (id int, d char(1))
SERVER loopback options (table_name 'conversions');
SET plan_cache_mode = force_generic_plan;
PREPARE s(varchar) AS SELECT count(*) FROM ft_conversions where d=$1;
EXPLAIN (VERBOSE, COSTS OFF)
EXECUTE s('1');
                                         QUERY PLAN
-------------------------------------------------------------------------------------------
  Foreign Scan
    Output: (count(*))
    Relations: Aggregate on (public.ft_conversions)
    Remote SQL: SELECT count(*) FROM public.conversions WHERE ((d = 
$1::character varying))
(4 rows)

EXECUTE s('1');
ERROR:  operator does not exist: public.enum_of_int_like = character 
varying
HINT:  No operator matches the given name and argument types. You might 
need to add explicit type casts.

> 2) Can we fallback to remote type conversion in local type conversion 
> fails?

It's the opposite - we've already planned (and deparsed) statement, 
using remote type conversion.
When plan execution fails, there's nothing we can do.
We'll get

PREPARE s(varchar[]) AS SELECT count(*) FROM ft_conversions where 
d=ANY($1);
EXPLAIN (VERBOSE, COSTS OFF)
EXECUTE s(ARRAY['1','2']);
                                             QUERY PLAN
---------------------------------------------------------------------------------------------------
  Foreign Scan
    Output: (count(*))
    Relations: Aggregate on (public.ft_conversions)
    Remote SQL: SELECT count(*) FROM public.conversions WHERE ((d = ANY 
($1::character varying[])))
(4 rows)

EXECUTE s(ARRAY['1','2']);
ERROR:  operator does not exist: public.enum_of_int_like = character 
varying
HINT:  No operator matches the given name and argument types. You might 
need to add explicit type casts.

-- 
Best regards,
Alexander Pyhalov,
Postgres Professional

  9. Re: postgres_fdw could deparse ArrayCoerceExpr

    Alexander Korotkov <aekorotkov@gmail.com> — 2025-06-04T20:52:50Z

    On Wed, Jun 4, 2025 at 6:15 PM Alexander Pyhalov
<a.pyhalov@postgrespro.ru> wrote:
> Alexander Korotkov писал(а) 2025-06-04 14:29:
> > On Wed, Jan 29, 2025 at 11:59 AM Maxim Orlov <orlovmg@gmail.com> wrote:
> >>
> >> One important note here. This patch will change cast behaviour in case
> >> of local and foreign types are mismatched.
> >> The problem is if we cannot convert types locally, this does not mean
> >> that it is also true for a foreign wrapped data.
> >> In any case, it's up to the committer to decide whether this change is
> >> needed or not.
> >
> > I have two question regarding this aspect.
> > 1) Is it the same with regular type conversion?
>
> Yes, it's the same.
>
> CREATE TYPE enum_of_int_like AS enum('1', '2', '3', '4');
> CREATE TABLE conversions(id int, d enum_of_int_like);
> CREATE FOREIGN TABLE ft_conversions (id int, d char(1))
> SERVER loopback options (table_name 'conversions');
> SET plan_cache_mode = force_generic_plan;
> PREPARE s(varchar) AS SELECT count(*) FROM ft_conversions where d=$1;
> EXPLAIN (VERBOSE, COSTS OFF)
> EXECUTE s('1');
>                                          QUERY PLAN
> -------------------------------------------------------------------------------------------
>   Foreign Scan
>     Output: (count(*))
>     Relations: Aggregate on (public.ft_conversions)
>     Remote SQL: SELECT count(*) FROM public.conversions WHERE ((d =
> $1::character varying))
> (4 rows)
>
> EXECUTE s('1');
> ERROR:  operator does not exist: public.enum_of_int_like = character
> varying
> HINT:  No operator matches the given name and argument types. You might
> need to add explicit type casts.
>
> > 2) Can we fallback to remote type conversion in local type conversion
> > fails?
>
> It's the opposite - we've already planned (and deparsed) statement,
> using remote type conversion.
> When plan execution fails, there's nothing we can do.
> We'll get
>
> PREPARE s(varchar[]) AS SELECT count(*) FROM ft_conversions where
> d=ANY($1);
> EXPLAIN (VERBOSE, COSTS OFF)
> EXECUTE s(ARRAY['1','2']);
>                                              QUERY PLAN
> ---------------------------------------------------------------------------------------------------
>   Foreign Scan
>     Output: (count(*))
>     Relations: Aggregate on (public.ft_conversions)
>     Remote SQL: SELECT count(*) FROM public.conversions WHERE ((d = ANY
> ($1::character varying[])))
> (4 rows)
>
> EXECUTE s(ARRAY['1','2']);
> ERROR:  operator does not exist: public.enum_of_int_like = character
> varying
> HINT:  No operator matches the given name and argument types. You might
> need to add explicit type casts.

Got it, thank you for the explanation.  I thin it's fair that array
coercion works the same way as a regular cast.

------
Regards,
Alexander Korotkov
Supabase

  10. Re: postgres_fdw could deparse ArrayCoerceExpr

    Alexander Korotkov <aekorotkov@gmail.com> — 2025-07-15T21:55:44Z

    On Wed, Jun 4, 2025 at 11:52 PM Alexander Korotkov <aekorotkov@gmail.com> wrote:
> On Wed, Jun 4, 2025 at 6:15 PM Alexander Pyhalov
> <a.pyhalov@postgrespro.ru> wrote:
> > Alexander Korotkov писал(а) 2025-06-04 14:29:
> > > On Wed, Jan 29, 2025 at 11:59 AM Maxim Orlov <orlovmg@gmail.com> wrote:
> > >>
> > >> One important note here. This patch will change cast behaviour in case
> > >> of local and foreign types are mismatched.
> > >> The problem is if we cannot convert types locally, this does not mean
> > >> that it is also true for a foreign wrapped data.
> > >> In any case, it's up to the committer to decide whether this change is
> > >> needed or not.
> > >
> > > I have two question regarding this aspect.
> > > 1) Is it the same with regular type conversion?
> >
> > Yes, it's the same.
> >
> > CREATE TYPE enum_of_int_like AS enum('1', '2', '3', '4');
> > CREATE TABLE conversions(id int, d enum_of_int_like);
> > CREATE FOREIGN TABLE ft_conversions (id int, d char(1))
> > SERVER loopback options (table_name 'conversions');
> > SET plan_cache_mode = force_generic_plan;
> > PREPARE s(varchar) AS SELECT count(*) FROM ft_conversions where d=$1;
> > EXPLAIN (VERBOSE, COSTS OFF)
> > EXECUTE s('1');
> >                                          QUERY PLAN
> > -------------------------------------------------------------------------------------------
> >   Foreign Scan
> >     Output: (count(*))
> >     Relations: Aggregate on (public.ft_conversions)
> >     Remote SQL: SELECT count(*) FROM public.conversions WHERE ((d =
> > $1::character varying))
> > (4 rows)
> >
> > EXECUTE s('1');
> > ERROR:  operator does not exist: public.enum_of_int_like = character
> > varying
> > HINT:  No operator matches the given name and argument types. You might
> > need to add explicit type casts.
> >
> > > 2) Can we fallback to remote type conversion in local type conversion
> > > fails?
> >
> > It's the opposite - we've already planned (and deparsed) statement,
> > using remote type conversion.
> > When plan execution fails, there's nothing we can do.
> > We'll get
> >
> > PREPARE s(varchar[]) AS SELECT count(*) FROM ft_conversions where
> > d=ANY($1);
> > EXPLAIN (VERBOSE, COSTS OFF)
> > EXECUTE s(ARRAY['1','2']);
> >                                              QUERY PLAN
> > ---------------------------------------------------------------------------------------------------
> >   Foreign Scan
> >     Output: (count(*))
> >     Relations: Aggregate on (public.ft_conversions)
> >     Remote SQL: SELECT count(*) FROM public.conversions WHERE ((d = ANY
> > ($1::character varying[])))
> > (4 rows)
> >
> > EXECUTE s(ARRAY['1','2']);
> > ERROR:  operator does not exist: public.enum_of_int_like = character
> > varying
> > HINT:  No operator matches the given name and argument types. You might
> > need to add explicit type casts.
>
> Got it, thank you for the explanation.  I thin it's fair that array
> coercion works the same way as a regular cast.

I've written a commit message for this patch.  I'm going to push this
if no objections.

------
Regards,
Alexander Korotkov
Supabase

  11. Re: postgres_fdw could deparse ArrayCoerceExpr

    Tender Wang <tndrwang@gmail.com> — 2025-07-18T14:33:50Z

    Alexander Korotkov <aekorotkov@gmail.com> 于2025年7月16日周三 05:56写道：

> On Wed, Jun 4, 2025 at 11:52 PM Alexander Korotkov <aekorotkov@gmail.com>
> wrote:
> > On Wed, Jun 4, 2025 at 6:15 PM Alexander Pyhalov
> > <a.pyhalov@postgrespro.ru> wrote:
> > > Alexander Korotkov писал(а) 2025-06-04 14:29:
> > > > On Wed, Jan 29, 2025 at 11:59 AM Maxim Orlov <orlovmg@gmail.com>
> wrote:
> > > >>
> > > >> One important note here. This patch will change cast behaviour in
> case
> > > >> of local and foreign types are mismatched.
> > > >> The problem is if we cannot convert types locally, this does not
> mean
> > > >> that it is also true for a foreign wrapped data.
> > > >> In any case, it's up to the committer to decide whether this change
> is
> > > >> needed or not.
> > > >
> > > > I have two question regarding this aspect.
> > > > 1) Is it the same with regular type conversion?
> > >
> > > Yes, it's the same.
> > >
> > > CREATE TYPE enum_of_int_like AS enum('1', '2', '3', '4');
> > > CREATE TABLE conversions(id int, d enum_of_int_like);
> > > CREATE FOREIGN TABLE ft_conversions (id int, d char(1))
> > > SERVER loopback options (table_name 'conversions');
> > > SET plan_cache_mode = force_generic_plan;
> > > PREPARE s(varchar) AS SELECT count(*) FROM ft_conversions where d=$1;
> > > EXPLAIN (VERBOSE, COSTS OFF)
> > > EXECUTE s('1');
> > >                                          QUERY PLAN
> > >
> -------------------------------------------------------------------------------------------
> > >   Foreign Scan
> > >     Output: (count(*))
> > >     Relations: Aggregate on (public.ft_conversions)
> > >     Remote SQL: SELECT count(*) FROM public.conversions WHERE ((d =
> > > $1::character varying))
> > > (4 rows)
> > >
> > > EXECUTE s('1');
> > > ERROR:  operator does not exist: public.enum_of_int_like = character
> > > varying
> > > HINT:  No operator matches the given name and argument types. You might
> > > need to add explicit type casts.
> > >
> > > > 2) Can we fallback to remote type conversion in local type conversion
> > > > fails?
> > >
> > > It's the opposite - we've already planned (and deparsed) statement,
> > > using remote type conversion.
> > > When plan execution fails, there's nothing we can do.
> > > We'll get
> > >
> > > PREPARE s(varchar[]) AS SELECT count(*) FROM ft_conversions where
> > > d=ANY($1);
> > > EXPLAIN (VERBOSE, COSTS OFF)
> > > EXECUTE s(ARRAY['1','2']);
> > >                                              QUERY PLAN
> > >
> ---------------------------------------------------------------------------------------------------
> > >   Foreign Scan
> > >     Output: (count(*))
> > >     Relations: Aggregate on (public.ft_conversions)
> > >     Remote SQL: SELECT count(*) FROM public.conversions WHERE ((d = ANY
> > > ($1::character varying[])))
> > > (4 rows)
> > >
> > > EXECUTE s(ARRAY['1','2']);
> > > ERROR:  operator does not exist: public.enum_of_int_like = character
> > > varying
> > > HINT:  No operator matches the given name and argument types. You might
> > > need to add explicit type casts.
> >
> > Got it, thank you for the explanation.  I thin it's fair that array
> > coercion works the same way as a regular cast.
>
> I've written a commit message for this patch.  I'm going to push this
> if no objections.
>

Hi Alexander,

I found a little typo in this commit. Other places use "an"
before  ArrayCoerceExpr.
To be consistent may be better. So, please take a look at the attached
patch.

-- 
Thanks,
Tender Wang

  12. Re: postgres_fdw could deparse ArrayCoerceExpr

    Alexander Korotkov <aekorotkov@gmail.com> — 2025-07-18T15:41:55Z

    On Fri, Jul 18, 2025 at 5:34 PM Tender Wang <tndrwang@gmail.com> wrote:
> Alexander Korotkov <aekorotkov@gmail.com> 于2025年7月16日周三 05:56写道：
>>
>> On Wed, Jun 4, 2025 at 11:52 PM Alexander Korotkov <aekorotkov@gmail.com> wrote:
>> > On Wed, Jun 4, 2025 at 6:15 PM Alexander Pyhalov
>> > <a.pyhalov@postgrespro.ru> wrote:
>> > > Alexander Korotkov писал(а) 2025-06-04 14:29:
>> > > > On Wed, Jan 29, 2025 at 11:59 AM Maxim Orlov <orlovmg@gmail.com> wrote:
>> > > >>
>> > > >> One important note here. This patch will change cast behaviour in case
>> > > >> of local and foreign types are mismatched.
>> > > >> The problem is if we cannot convert types locally, this does not mean
>> > > >> that it is also true for a foreign wrapped data.
>> > > >> In any case, it's up to the committer to decide whether this change is
>> > > >> needed or not.
>> > > >
>> > > > I have two question regarding this aspect.
>> > > > 1) Is it the same with regular type conversion?
>> > >
>> > > Yes, it's the same.
>> > >
>> > > CREATE TYPE enum_of_int_like AS enum('1', '2', '3', '4');
>> > > CREATE TABLE conversions(id int, d enum_of_int_like);
>> > > CREATE FOREIGN TABLE ft_conversions (id int, d char(1))
>> > > SERVER loopback options (table_name 'conversions');
>> > > SET plan_cache_mode = force_generic_plan;
>> > > PREPARE s(varchar) AS SELECT count(*) FROM ft_conversions where d=$1;
>> > > EXPLAIN (VERBOSE, COSTS OFF)
>> > > EXECUTE s('1');
>> > >                                          QUERY PLAN
>> > > -------------------------------------------------------------------------------------------
>> > >   Foreign Scan
>> > >     Output: (count(*))
>> > >     Relations: Aggregate on (public.ft_conversions)
>> > >     Remote SQL: SELECT count(*) FROM public.conversions WHERE ((d =
>> > > $1::character varying))
>> > > (4 rows)
>> > >
>> > > EXECUTE s('1');
>> > > ERROR:  operator does not exist: public.enum_of_int_like = character
>> > > varying
>> > > HINT:  No operator matches the given name and argument types. You might
>> > > need to add explicit type casts.
>> > >
>> > > > 2) Can we fallback to remote type conversion in local type conversion
>> > > > fails?
>> > >
>> > > It's the opposite - we've already planned (and deparsed) statement,
>> > > using remote type conversion.
>> > > When plan execution fails, there's nothing we can do.
>> > > We'll get
>> > >
>> > > PREPARE s(varchar[]) AS SELECT count(*) FROM ft_conversions where
>> > > d=ANY($1);
>> > > EXPLAIN (VERBOSE, COSTS OFF)
>> > > EXECUTE s(ARRAY['1','2']);
>> > >                                              QUERY PLAN
>> > > ---------------------------------------------------------------------------------------------------
>> > >   Foreign Scan
>> > >     Output: (count(*))
>> > >     Relations: Aggregate on (public.ft_conversions)
>> > >     Remote SQL: SELECT count(*) FROM public.conversions WHERE ((d = ANY
>> > > ($1::character varying[])))
>> > > (4 rows)
>> > >
>> > > EXECUTE s(ARRAY['1','2']);
>> > > ERROR:  operator does not exist: public.enum_of_int_like = character
>> > > varying
>> > > HINT:  No operator matches the given name and argument types. You might
>> > > need to add explicit type casts.
>> >
>> > Got it, thank you for the explanation.  I thin it's fair that array
>> > coercion works the same way as a regular cast.
>>
>> I've written a commit message for this patch.  I'm going to push this
>> if no objections.
>
>
> Hi Alexander,
>
> I found a little typo in this commit. Other places use "an" before  ArrayCoerceExpr.
> To be consistent may be better. So, please take a look at the attached patch.

Sure thing, pushed!

------
Regards,
Alexander Korotkov
Supabase