Re: Add proper planner support for ORDER BY / DISTINCT aggregates
| От | David Rowley |
|---|---|
| Тема | Re: Add proper planner support for ORDER BY / DISTINCT aggregates |
| Дата | |
| Msg-id | CAApHDvqS-p57j++gLMyQXmttk_MM3u243dWQ54FyW-7=4GP+1Q@mail.gmail.com обсуждение исходный текст |
| Ответ на | Re: Add proper planner support for ORDER BY / DISTINCT aggregates (Ronan Dunklau <ronan.dunklau@aiven.io>) |
| Ответы |
Re: Add proper planner support for ORDER BY / DISTINCT aggregates
|
| Список | pgsql-hackers |
On Thu, 4 Nov 2021 at 20:59, Ronan Dunklau <ronan.dunklau@aiven.io> wrote: > I took some time to toy with this again. > > At first I thought that charging a discount in foreign grouping paths for > Aggref targets (since they are computed remotely) would be a good idea, > similar to what is done for the grouping keys. I've been working on this patch again. There was a bit of work to do to rebase it atop db0d67db2. The problem there was that since this patch appends pathkeys to suit ORDER BY / DISTINCT aggregates to the query's group_pathkeys, db0d67db2 goes and tries to rearrange those, but fails to find the SortGroupClause corresponding to the PathKey in group_pathkeys. I wish the code I came up with to make that work was a bit nicer, but what's there at least seems to work. There are a few more making copies of Lists than I'd like. I've also went and added LLVM support to make JIT work with the new DISTINCT expression evaluation step types. Also, James mentioned in [1] about the Merge Join plan change that this patch was causing in an existing test. I looked into that and found the cause. The plan change is not really the fault of this patch, so I've proposed a fix for to make that work more efficiently in [2]. The basics there are that select_outer_pathkeys_for_merge() pre-dates Incremental Sorts and didn't consider prefixes of the query_pathkeys after matching all the join quals. The patch on that thread relaxes that rule and makes this patch produce an Incremental Sort plan for the query in question. Another annoying part of this patch is that I've added an "aggpresorted" field to Aggref, which the planner sets. That's a parse node type and it would be nicer not to have the planner mess around with those. We maybe could wrap up the Aggrefs in some planner struct and pass those to the executor instead. That would require a bit more churn than what I've got in the attached. I've attached the v7 patch. David [1] https://www.postgresql.org/message-id/CAAaqYe-yxXkXVPJkRw1nDA=CJBw28jvhACRyDcU10dAOcdSj=Q@mail.gmail.com [2] https://www.postgresql.org/message-id/CAApHDvrtZu0PHVfDPFM4Yx3jNR2Wuwosv+T2zqa7LrhhBr2rRg@mail.gmail.com
Вложения
В списке pgsql-hackers по дате отправления: