| 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: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| 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.
Regards,
--
Ronan Dunklau
| Attachment | Content-Type | Size |
|---|---|---|
| v4_fix_postgresfdw_orderby_handling.txt | text/plain | 16.2 KB |
| 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 |