On Sat, Jul 28, 2018 at 4:12 AM, Alexander Kuzmenkov <a.kuzmen...@postgrespro.ru> wrote: > Daniel, > > Thanks for the update. > > > On 07/25/2018 01:37 AM, Daniel Gustafsson wrote: >> >> >>> Hmm, this is missing the eqop fields of SortGroupClause. I haven't >>> tested yet but does the similar degradation happen if two >>> SortGroupCaluses have different eqop and the same other values? >> >> I wasn’t able to construct a case showing this, but I also think you’re >> right. >> Do you have an idea of a query that can trigger a regression? The >> attached >> patch adds a stab at this, but I’m not sure if it’s the right approach. > > > To trigger that, in your test example you could order by empno::int8 for one > window, and by empno::int2 for another. But don't I think you have to > compare eqop here, because if eqop differs, sortop will differ too. I > removed the comparison from the patch. I also clarified (I hope) the > comments, and did the optimization I mentioned earlier: using array instead > of list for active clauses. Please see the attached v6. Otherwise I think > the patch is good. >
Thank you! That makes sense and the patch looks good to me. FWIW maybe it's good idea to add the comment describing why we didn't that. Regards, -- Masahiko Sawada NIPPON TELEGRAPH AND TELEPHONE CORPORATION NTT Open Source Software Center
