Re: ORDER BY pushdowns seem broken in postgres_fdw

Ronan Dunklau <ronan.dunklau@aiven.io>

From: Ronan Dunklau <ronan.dunklau@aiven.io>
To: Ranier Vilela <ranier.vf@gmail.com>
Cc: David Rowley <dgrowleyml@gmail.com>, PostgreSQL Developers <pgsql-hackers@lists.postgresql.org>, Robert Haas <robertmhaas@gmail.com>, Jeevan Chalke <jeevan.chalke@enterprisedb.com>, Ashutosh Bapat <ashutosh.bapat.oss@gmail.com>
Date: 2021-07-22T07:00:20Z
Lists: pgsql-hackers

Attachments

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.



Regards,


-- 
Ronan Dunklau

Commits

  1. Fix postgres_fdw to check shippability of sort clauses properly.

  2. postgres_fdw: Add ORDER BY to some remote SQL queries.