Re: A rather hackish POC for alternative implementation of WITH TIES

From: Surafel Temesgen <surafel3000(at)gmail(dot)com>
To: Andrew Gierth <andrew(at)tao11(dot)riddles(dot)org(dot)uk>
Cc: PostgreSQL Hackers <pgsql-hackers(at)postgresql(dot)org>, Alvaro Herrera <alvherre(at)2ndquadrant(dot)com>, Tomas Vondra <tomas(dot)vondra(at)2ndquadrant(dot)com>
Subject: Re: A rather hackish POC for alternative implementation of WITH TIES
Date: 2020-01-06 08:20:56
Message-ID: CALAY4q8KPZz+TtmuL_9MODDH7E=i7MSbknE93ukT+d7TBwFF0A@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Fri, Nov 29, 2019 at 8:40 AM Andrew Gierth <andrew(at)tao11(dot)riddles(dot)org(dot)uk>
wrote:

> This patch is a rather hacky implementation of the basic idea for
> implementing FETCH ... WITH TIES, and potentially also PERCENT, by using
> a window function expression to compute a stopping point.
>
> Large chunks of this (the parser/ruleutils changes, docs, tests) are
> taken from Surafel Temesgen's patch. The difference is that the executor
> change in my version is minimal: Limit allows a boolean column in the
> input to signal the point at which to stop. The planner inserts a
> WindowAgg node to compute the necessary condition using the rank()
> function.
>
> The way this is done in the planner isn't (IMO) the best and should
> probably be improved; in particular it currently misses some possible
> optimizations (most notably constant-folding of the offset+limit
> subexpression). I also haven't tested it properly to see whether I broke
> anything, though it does pass regression.
>
>
>
Unlike most other executor node limit node has implementation for handling
backward scan that support cursor operation but your approach didn't do
this inherently because it outsource limitNode functionality to window
function and window function didn't do this

eg.

postgres=# begin;

BEGIN

postgres=# declare c cursor for select i from generate_series(1,1000000)
s(i) order by i fetch first 2 rows with ties;

DECLARE CURSOR

postgres=# fetch all in c;

i

---

1

2

(2 rows)

postgres=# fetch backward all in c;

ERROR: cursor can only scan forward

HINT: Declare it with SCROLL option to enable backward scan.

Even with SCROLL option it is not working as limitNode does. It store the
result and return in backward scan that use more space than current limit
and limit with ties implementation.

If am not mistaken the patch also reevaluate limit every time returning row
beside its not good for performance its will return incorrect result with
limit involving volatile function

regards

Surafel

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Julien Rouhaud 2020-01-06 08:31:28 Re: pg_basebackup fails on databases with high OIDs
Previous Message Michael Paquier 2020-01-06 08:20:53 Re: pg_basebackup fails on databases with high OIDs