From: | Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)com> |
---|---|
To: | Daniel Gustafsson <daniel(at)yesql(dot)se> |
Cc: | Alexander Kuzmenkov <a(dot)kuzmenkov(at)postgrespro(dot)ru>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org> |
Subject: | Re: Avoid extra Sort nodes between WindowAggs when sorting can be reused |
Date: | 2018-07-03 10:24:06 |
Message-ID: | CAD21AoD254542kbWMJsH_jC44r1haXB1PqZyohBRi+fMnwJq=w@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
On Tue, Jul 3, 2018 at 6:19 AM, Daniel Gustafsson <daniel(at)yesql(dot)se> wrote:
>> On 2 Jul 2018, at 14:01, Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)com> wrote:
>
>> Thank you for updating the patch! There are two review comments.
>
> Thanks for reviewing!
>
>> The current select_active_windows() function compares the all fields
>> of WindowClause for the sorting but with this patch we compare only
>> tleSortGroupRef, sortop and the number of uniqueOrder. I think this
>> leads a degradation as follows.
>
> You are right, that was an oversight. The attached patch takes a stab at
> fixing this.
>
>> s/readibility/readability/
>
> Fixed.
Thank you for updating the patch.
+ if (sca->tleSortGroupRef > scb->tleSortGroupRef)
+ return -1;
+ else if (sca->tleSortGroupRef < scb->tleSortGroupRef)
+ return 1;
+ else if (sca->sortop > scb->sortop)
+ return -1;
+ else if (sca->sortop < scb->sortop)
+ return 1;
+ else if (sca->nulls_first && !scb->nulls_first)
+ return -1;
+ else if (!sca->nulls_first && scb->nulls_first)
+ return 1;
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?
--
The source code comments for common_prefix_cmp() function and
WindowClauseSortNode struct is needed.
--
+-- Test Sort node reordering
+EXPLAIN (COSTS OFF)
+SELECT
+ lead(1) OVER (PARTITION BY depname ORDER BY salary, enroll_date),
+ lag(1) OVER (PARTITION BY depname ORDER BY salary,enroll_date,empno)
+ from empsalary;
I think it's better to change "from empsalary" to "FROM empsalary" for
consistency with other code.
Regards,
--
Masahiko Sawada
NIPPON TELEGRAPH AND TELEPHONE CORPORATION
NTT Open Source Software Center
From | Date | Subject | |
---|---|---|---|
Next Message | s.cherkashin | 2018-07-03 10:25:37 | Re: Psql patch to show access methods info |
Previous Message | ROS Didier | 2018-07-03 10:17:13 | How to use public key file to encrypt data |