>>>>> "Surafel" == Surafel Temesgen <surafel3...@gmail.com> writes:
Surafel> Unlike most other executor node limit node has implementation Surafel> for handling backward scan that support cursor operation but Surafel> your approach didn't do this inherently because it outsource Surafel> limitNode functionality to window function and window function Surafel> didn't do this Correct. But this is a non-issue: if you want to be able to do backward scan you are supposed to declare the cursor as SCROLL; if it happens to work without it, that is pure coincidence. (Cursors declared with neither SCROLL nor NO SCROLL support backwards scan only if the underlying plan supports backward scan with no additional overhead, which is something you can't predict from the query.) The Limit node declares that it supports backwards scan if, and only if, its immediate child node supports it. It happens that WindowAgg does not, so in this implementation, LIMIT ... WITH TIES will not support backward scan without a tuplestore. I don't consider this an especially big deal; backward scans are extremely rare (as shown by the fact that bugs in backward scan have tended to go unnoticed for decades, e.g. bug #15336), and therefore we should not optimize for them. Surafel> If am not mistaken the patch also reevaluate limit every time The (offset+limit) expression is, yes. I noted in the original post that this needs work - probably it should be pushed out to an InitPlan if it doesn't fold to a constant. i.e. using the expression rank() over (...) > (select offset+limit) where it currently has rank() over (...) > (offset+limit) (Generating the limit expression so late in planning is the main thing that needs changing to get this from a hack POC to usable code) The main point here is that the same rather minimal executor changes allow support for not only WITH TIES but also PERCENT and possibly arbitrary stop conditions as well. (I know I've often wanted LIMIT WHEN to stop a query at a data-dependent point without having to resort to recursion - this patch doesn't quite get there, because of the scope issues involved in analyzing the WHEN condition, but it at least sets up the concept.) -- Andrew (irc:RhodiumToad)