Re: ORDER BY pushdowns seem broken in postgres_fdw

From: Ronan Dunklau <ronan(dot)dunklau(at)aiven(dot)io>
To: Ranier Vilela <ranier(dot)vf(at)gmail(dot)com>
Cc: David Rowley <dgrowleyml(at)gmail(dot)com>, 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>
Subject: Re: ORDER BY pushdowns seem broken in postgres_fdw
Date: 2021-07-22 07:00:20
Message-ID: 3165278.bG4bnikssK@aivenronan
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

Le jeudi 22 juillet 2021, 02:16:52 CEST Ranier Vilela a écrit :
> Unfortunately your patch does not apply clear into the head.
> So I have a few suggestions on v2, attached with the .txt extension to
> avoid cf bot.
> Please, if ok, make the v3.

Hum weird, it applied cleanly for me, and was formatted using git show which I
admit is not ideal. Please find it reattached.

> 2. appendOrderbyUsingClause function
> Put the buffer actions together?
Not sure what you mean here ?

> 3. Apply style Postgres?
> + if (!HeapTupleIsValid(tuple))
> + {
> + elog(ERROR, "cache lookup failed for operator family %u",
> pathkey->pk_opfamily);
> + }

Good catch !

> 4. Assertion not ok here?
> + em = find_em_for_rel(pathkey->pk_eclass, baserel);
> + em_expr = em->em_expr;
> Assert(em_expr != NULL);

If we are here there should never be a case where the em can't be found. I
moved the assertion where it makes sense though.


Ronan Dunklau

Attachment Content-Type Size
v4_fix_postgresfdw_orderby_handling.txt text/plain 16.2 KB

In response to


Browse pgsql-hackers by date

  From Date Subject
Next Message Julien Rouhaud 2021-07-22 07:04:19 Re: Hook for extensible parsing.
Previous Message Pavel Stehule 2021-07-22 06:33:09 Re: psql - add SHOW_ALL_RESULTS option