Re: Invalid query generated by postgres_fdw with UNION ALL and ORDER BY

Поиск
Список
Период
Сортировка
От Ashutosh Bapat
Тема Re: Invalid query generated by postgres_fdw with UNION ALL and ORDER BY
Дата
Msg-id CAExHW5u7sJbkUV+eHV2NRmSYXuxUz=widBF6D+TufgLF8-YV9Q@mail.gmail.com
обсуждение исходный текст
Ответ на Re: Invalid query generated by postgres_fdw with UNION ALL and ORDER BY  (David Rowley <dgrowleyml@gmail.com>)
Список pgsql-hackers


On Fri, Mar 8, 2024 at 7:43 AM David Rowley <dgrowleyml@gmail.com> wrote:
On Fri, 8 Mar 2024 at 00:54, Ashutosh Bapat
<ashutosh.bapat.oss@gmail.com> wrote:
>
> On Thu, Mar 7, 2024 at 4:39 PM David Rowley <dgrowleyml@gmail.com> wrote:
>> 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 = 1. appendOrderByClause() may want to do something similar for consistency. Or remove it from deparseSortGroupClause() as well?

The fix could also be to use deparseConst() in appendOrderByClause()
and have that handle Const EquivalenceMember instead.  I'd rather just
skip them. To me, that seems less risky than ensuring deparseConst()
handles all Const types correctly.

Also, as far as adjusting GROUP BY to eliminate Consts, I don't think
that's material for a bug fix. If we want to consider doing that,
that's for master only.

If appendOrderByClause() would have been using deparseConst() since the beginning this bug would not be there. Instead of maintaining two different ways of deparsing ORDER BY clause, we could maintain just one. I think we should unify those. If we should do it in only master be it so. I am fine to leave back branches with two methods.
 

>> I wonder if we need a test...
>
> Yes.

I've added two of those in the attached.

Thanks. They look fine to me.


--
Best Wishes,
Ashutosh Bapat

В списке pgsql-hackers по дате отправления:

Предыдущее
От: Amit Kapila
Дата:
Сообщение: Re: Introduce XID age and inactive timeout based replication slot invalidation
Следующее
От: Peter Smith
Дата:
Сообщение: Re: Improve eviction algorithm in ReorderBuffer