Re: ORDER BY pushdowns seem broken in postgres_fdw

From: Ronan Dunklau <ronan(dot)dunklau(at)aiven(dot)io>
To: PostgreSQL Developers <pgsql-hackers(at)lists(dot)postgresql(dot)org>, Robert Haas <robertmhaas(at)gmail(dot)com>, Jeevan Chalke <jeevan(dot)chalke(at)enterprisedb(dot)com>, Ashutosh Bapat <ashutosh(dot)bapat(dot)oss(at)gmail(dot)com>
Cc: David Rowley <dgrowleyml(at)gmail(dot)com>
Subject: Re: ORDER BY pushdowns seem broken in postgres_fdw
Date: 2021-07-21 12:28:30
Message-ID: 5578227.HAYmEhll7C@aivenronan
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

Le mercredi 21 juillet 2021, 11:05:14 CEST Ronan Dunklau a écrit :
> Le mercredi 21 juillet 2021, 04:25:00 CEST David Rowley a écrit :
> > Here the test claims that it wants to ensure that the order by using
> > operator(public.<^) is not pushed down into the foreign scan.
> > However, unless I'm mistaken, it seems there's a completely wrong
> > assumption there that the planner would even attempt that. In current
> > master we don't add PathKeys for ORDER BY aggregates, why would that
> > sort get pushed down in the first place?
> The whole aggregate, including it's order by clause, can be pushed down so
> there is nothing related to pathkeys here.
> > If I adjust that query to something that would have the planner set
> > pathkeys for, it does push the ORDER BY to the foreign server without
> > any consideration that the sort operator is not shippable to the
> > foreign server.
> >
> > Am I missing something here, or is postgres_fdw.c's
> > get_useful_pathkeys_for_relation() just broken?
> I think you're right, we need to add a check if the opfamily is shippable.
> I'll submit a patch for that including regression tests.

In fact the support for generating the correct USING clause was inexistent
too, so that needed a bit more work.

The attached patch does the following:
- verify the opfamily is shippable to keep pathkeys
- generate a correct order by clause using the actual operator.

The second part needed a bit of refactoring: the find_em_expr_for_input_target
and find_em_expr_for_rel need to return the whole EquivalenceMember, because we
can't know the type used by the opfamily from the expression (example: the
expression could be of type intarray, but the type used by the opfamily could
be anyarray).

I also moved the "USING <operator>"' string generation to a separate function
since it's now used by appendAggOrderBy and appendOrderByClause.

The find_em_expr_for_rel is exposed in optimizer/paths.h, so I kept the
existing function which returns the expr directly in case it is used out of

Ronan Dunklau

Attachment Content-Type Size
v1_fix_postgresfdw_orderby_handling text/plain 15.1 KB

In response to


Browse pgsql-hackers by date

  From Date Subject
Next Message Ranier Vilela 2021-07-21 12:28:42 Re: Micro-optimizations to avoid some strlen calls.
Previous Message Michael Paquier 2021-07-21 11:49:45 Re: Incorrect usage of strtol, atoi for non-numeric junk inputs