MIME-Version: 1.0
References: <0714C8B8-8D82-4ABB-9F8D-A0C3657E7B6E@kleczek.org> <CAApHDvpfyTA3Pnf_P6Kx8s5rNSUBxPfsZxiFz6DpUyMiwqHERw@mail.gmail.com>
In-Reply-To: <CAApHDvpfyTA3Pnf_P6Kx8s5rNSUBxPfsZxiFz6DpUyMiwqHERw@mail.gmail.com>
From: Ashutosh Bapat <ashutosh.bapat.oss@gmail.com>
Date: Thu, 7 Mar 2024 17:24:40 +0530
Message-ID: <CAExHW5taz6hX05P55_S1cbx+P5CcNN21rD7oLY5iXJcShrAbaQ@mail.gmail.com>
Subject: Re: Invalid query generated by postgres_fdw with UNION ALL and ORDER BY
To: David Rowley <dgrowleyml@gmail.com>
Cc: =?UTF-8?B?TWljaGHFgiBLxYJlY3plaw==?= <michal@kleczek.org>, 
	PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
Content-Type: multipart/alternative; boundary="000000000000375a63061310be71"
Archived-At: <https://www.postgresql.org/message-id/CAExHW5taz6hX05P55_S1cbx%2BP5CcNN21rD7oLY5iXJcShrAbaQ%40mail.gmail.com>
Precedence: bulk

--000000000000375a63061310be71
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

On Thu, Mar 7, 2024 at 4:39=E2=80=AFPM David Rowley <dgrowleyml@gmail.com> =
wrote:

> On Thu, 7 Mar 2024 at 19:09, Micha=C5=82 K=C5=82eczek <michal@kleczek.org=
> wrote:
> >
> > The following query:
> >
> > SELECT * FROM (
> >   SELECT 2023 AS year, * FROM remote_table_1
> >   UNION ALL
> >   SELECT 2022 AS year, * FROM remote_table_2
> > )
> > ORDER BY year DESC;
> >
> > yields the following remote query:
> >
> > SELECT [columns] FROM remote_table_1 ORDER BY 2023 DESC
> >
> > and subsequently fails remote execution.
> >
> >
> > Not really sure where the problem is - the planner or postgres_fdw.
> > I guess it is postgres_fdw not filtering out ordering keys.
>
> Interesting.  I've attached a self-contained recreator for the casual
> passerby.
>
> I think the fix should go in appendOrderByClause().  It's at that
> point we look for the EquivalenceMember for the relation and can
> easily discover if the em_expr is a Const.  I think we can safely just
> skip doing any ORDER BY <const> stuff and not worry about if the
> literal format of the const will appear as a reference to an ordinal
> column position in the ORDER BY clause.
>

deparseSortGroupClause() calls deparseConst() with showtype =3D 1.
appendOrderByClause() may want to do something similar for consistency. Or
remove it from deparseSortGroupClause() as well?


>
> Something like the attached patch I think should work.
>
> I wonder if we need a test...
>

Yes.

--=20
Best Wishes,
Ashutosh Bapat

--000000000000375a63061310be71
Content-Type: text/html; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

<div dir=3D"ltr"><div dir=3D"ltr"><br></div><br><div class=3D"gmail_quote">=
<div dir=3D"ltr" class=3D"gmail_attr">On Thu, Mar 7, 2024 at 4:39=E2=80=AFP=
M David Rowley &lt;<a href=3D"mailto:dgrowleyml@gmail.com">dgrowleyml@gmail=
.com</a>&gt; wrote:<br></div><blockquote class=3D"gmail_quote" style=3D"mar=
gin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1=
ex">On Thu, 7 Mar 2024 at 19:09, Micha=C5=82 K=C5=82eczek &lt;<a href=3D"ma=
ilto:michal@kleczek.org" target=3D"_blank">michal@kleczek.org</a>&gt; wrote=
:<br>
&gt;<br>
&gt; The following query:<br>
&gt;<br>
&gt; SELECT * FROM (<br>
&gt;=C2=A0 =C2=A0SELECT 2023 AS year, * FROM remote_table_1<br>
&gt;=C2=A0 =C2=A0UNION ALL<br>
&gt;=C2=A0 =C2=A0SELECT 2022 AS year, * FROM remote_table_2<br>
&gt; )<br>
&gt; ORDER BY year DESC;<br>
&gt;<br>
&gt; yields the following remote query:<br>
&gt;<br>
&gt; SELECT [columns] FROM remote_table_1 ORDER BY 2023 DESC<br>
&gt;<br>
&gt; and subsequently fails remote execution.<br>
&gt;<br>
&gt;<br>
&gt; Not really sure where the problem is - the planner or postgres_fdw.<br=
>
&gt; I guess it is postgres_fdw not filtering out ordering keys.<br>
<br>
Interesting.=C2=A0 I&#39;ve attached a self-contained recreator for the cas=
ual passerby.<br>
<br>
I think the fix should go in appendOrderByClause().=C2=A0 It&#39;s at that<=
br>
point we look for the EquivalenceMember for the relation and can<br>
easily discover if the em_expr is a Const.=C2=A0 I think we can safely just=
<br>
skip doing any ORDER BY &lt;const&gt; stuff and not worry about if the<br>
literal format of the const will appear as a reference to an ordinal<br>
column position in the ORDER BY clause.<br></blockquote><div><br></div><div=
>deparseSortGroupClause() calls deparseConst() with showtype =3D 1. appendO=
rderByClause() may want to do something similar for consistency. Or remove =
it from deparseSortGroupClause() as well?<br></div><div>=C2=A0</div><blockq=
uote class=3D"gmail_quote" style=3D"margin:0px 0px 0px 0.8ex;border-left:1p=
x solid rgb(204,204,204);padding-left:1ex">
<br>
Something like the attached patch I think should work.<br>
<br>
I wonder if we need a test...<br clear=3D"all"></blockquote><div><br></div>=
<div>Yes. <br></div></div><br><span class=3D"gmail_signature_prefix">-- </s=
pan><br><div dir=3D"ltr" class=3D"gmail_signature"><div dir=3D"ltr"><div><d=
iv dir=3D"ltr"><div>Best Wishes,</div><div>Ashutosh Bapat<br></div></div></=
div></div></div></div>

--000000000000375a63061310be71--