Message-ID: <2040c902-32b1-46c8-0b26-d3f43a16b9bb@gmail.com>
Date: Sun, 8 Jan 2023 11:48:05 +0530
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101
 Thunderbird/102.4.2
Subject: Re: Todo: Teach planner to evaluate multiple windows in the optimal
 order
Content-Language: en-US
To: David Rowley <dgrowleyml@gmail.com>
Cc: pghackers <pgsql-hackers@lists.postgresql.org>,
 Vik Fearing <vik@postgresfriends.org>
References: <83d80853-a45c-d85c-68eb-59acfe7fb5fb@gmail.com>
 <CAApHDvp=r1LnEKCmWCYaruMPL-jP4j_sdc8yeFYwaDT1ac5GsQ@mail.gmail.com>
 <7f77ee7d-bd04-d8e2-bb34-42395fd1f7c2@gmail.com>
 <CAApHDvq=g2=ny59f1bvwRVvupsgPHK-KjLPBsSL25fVuGZ4idQ@mail.gmail.com>
 <6ebcc137-45ea-6373-e3e1-3c304c452e1f@gmail.com>
 <441d135e-1941-c3ef-1649-18c3e8811549@postgresfriends.org>
 <9620d994-b89a-2dcf-fca5-821e19d56858@gmail.com>
 <CAApHDvruS-3gG_KYXPb_=Um_TfUXi+G4o7VyBVbChTjThXF2yA@mail.gmail.com>
 <a58a04a9-2dd6-7290-35c5-e3e23f326b85@gmail.com>
 <01248493-182b-0f92-f2f2-ff28dc4b2e83@gmail.com>
 <CAApHDvpEYcQ7d-c6Lje26gLi4dXOM3qQ9nrTe1Z241tT8nBiCg@mail.gmail.com>
 <c1d1aace-ce9f-9ec3-1984-5319920b1594@gmail.com>
 <CAApHDvp8BpUEo_kQdGHWNPCjcmRWCdiy5p26SoQA4R6rinkaLA@mail.gmail.com>
From: Ankit Kumar Pandey <itsankitkp@gmail.com>
In-Reply-To: <CAApHDvp8BpUEo_kQdGHWNPCjcmRWCdiy5p26SoQA4R6rinkaLA@mail.gmail.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit
Archived-At: <https://www.postgresql.org/message-id/2040c902-32b1-46c8-0b26-d3f43a16b9bb%40gmail.com>
Precedence: bulk


On 08/01/23 03:56, David Rowley wrote:

 > (your email client still seems broken)

I am looking at this again, will be changing client for here onward.

> You might need to have another loop before the foreach loop that loops
> backwards through the WindowClauses and remembers the index of the
> WindowClause which has pathkeys contained in the query's ORDER BY
> pathkeys then apply the optimisation from that point in the main
> foreach loop.  Also, if the condition within the foreach loop which
> checks when we want to apply this optimisation is going to be run > 1
> time, then you should probably have  boolean variable that's set
> before the loop which saves if we're going to try to apply the
> optimisation.  That'll save from having to check things like if the
>  query has a LIMIT clause multiple times.

Thanks, this should do the trick.

> a) looks like the best plan to me.  What's the point of pushing the
> sort below the WindowAgg in this case? The point of this optimisation
> is to reduce the number of sorts not to push them as deep into the
> plan as possible. We should only be pushing them down when it can
> reduce the number of sorts. There's no reduction in the number of
> sorts in the above plan.

Yes, you are right, not in this case. I actually mentioned wrong case here,

real problematic case is:

EXPLAIN (COSTS OFF)
SELECT empno,
        depname,
        min(salary) OVER (PARTITION BY depname ORDER BY empno) depminsalary,
        sum(salary) OVER (PARTITION BY depname) depsalary
FROM empsalary
ORDER BY depname, empno, enroll_date;
                             QUERY PLAN
-------------------------------------------------------------------
  Incremental Sort
    Sort Key: depname, empno, enroll_date
    Presorted Key: depname, empno
    ->  WindowAgg
          ->  WindowAgg
                ->  Incremental Sort
                      Sort Key: depname, empno
                      Presorted Key: depname
                      ->  Index Scan using depname_idx on empsalary
(9 rows)

Here, it could have sorted on depname, empno, enroll_date.

Again, as I mentioned before, this is implementation issue. We shouldn't be

skipping optimization if pre-sorted keys are present.

-- 
Regards,
Ankit Kumar Pandey