Hi, Andrei! On Thu, Apr 18, 2024 at 11:54 AM Andrei Lepikhov <a.lepik...@postgrespro.ru> wrote: > On 4/12/24 06:44, Tom Lane wrote: > > * Speaking of pathnodes.h, PathKeyInfo is a pretty uninformative node > > type name, and the comments provided for it are not going to educate > > anybody. What is the "association" between the pathkeys and clauses? > > I guess the clauses are supposed to be SortGroupClauses, but not all > > pathkeys match up to SortGroupClauses, so what then? This is very > > underspecified, and fuzzy thinking about data structures tends to lead > > to bugs. > I'm not the best in English documentation and naming style. So, feel > free to check my version. > > > > So I'm quite afraid that there are still bugs lurking here. > > What's more, given that the plans this patch makes apparently > > seldom win when you don't put a thumb on the scales, it might > > take a very long time to isolate those bugs. If the patch > > produces a faulty plan (e.g. not sorted properly) we'd never > > notice if that plan isn't chosen, and even if it is chosen > > it probably wouldn't produce anything as obviously wrong as > > a crash. > I added checkings on the proper order of pathkeys and clauses. > If you really care about that, we should spend additional time and > rewrite the code to generate an order of clauses somewhere in the plan > creation phase. For example, during the create_group_plan call, we could > use the processed_groupClause list, cycle through subpath->pathkeys, set > the order, and detect whether the pathkeys list corresponds to the > group-by or is enough to build a grouping plan. > Anyway, make this part of code more resistant to blunders is another story.
Thank you for the fixes you've proposed. I didn't look much into details yet, but I think the main concern Tom expressed in [1] is whether the feature is reasonable at all. I think at this stage the most important thing is to come up with convincing examples showing how huge performance benefits it could cause. I will return to this later today and will try to provide some convincing examples. Links 1. https://www.postgresql.org/message-id/266850.1712879082%40sss.pgh.pa.us ------ Regards, Alexander Korotkov
