Re: FETCH FIRST clause WITH TIES option - Mailing list pgsql-hackers

From Surafel Temesgen
Subject Re: FETCH FIRST clause WITH TIES option
Date
Msg-id CALAY4q82MDk1cehn-n2sSPu0iOLttXGk4A_CdXHnUSnQpK5sFA@mail.gmail.com
Whole thread Raw
In response to Re: FETCH FIRST clause WITH TIES option  (Alvaro Herrera <alvherre@2ndquadrant.com>)
Responses Re: FETCH FIRST clause WITH TIES option  (Alvaro Herrera <alvherre@2ndquadrant.com>)
List pgsql-hackers


On Thu, Nov 28, 2019 at 12:36 AM Alvaro Herrera <alvherre@2ndquadrant.com> wrote:
Thanks.

(I would suggest renaming the new state LIMIT_WINDOWEND_TIES, because it
seems to convey what it does a little better.)

I think you should add a /* fall-though */ comment after changing state.
Like this (this flow seems clearer; also DRY):

                if (!node->noCount &&
                    node->position - node->offset >= node->count)
                {
                    if (node->limitOption == LIMIT_OPTION_COUNT)
                    {
                        node->lstate = LIMIT_WINDOWEND;
                        return NULL;
                    }
                    else
                    {
                        node->lstate = LIMIT_WINDOWEND_TIES;
                        /* fall-through */
                    }
                }
                else
                    ...

 
changed
 
I've been playing with this a little more, and I think you've overlooked
a few places in ExecLimit that need to be changed.  In the regression
database, I tried this:

55432 13devel 17282=# begin;
BEGIN
55432 13devel 17282=# declare c1 cursor for select * from int8_tbl order by q1 fetch first 2 rows with ties;
DECLARE CURSOR
55432 13devel 17282=# fetch all in c1;
 q1  │        q2       
─────┼──────────────────
 123 │              456
 123 │ 4567890123456789
(2 filas)

55432 13devel 17282=# fetch all in c1;
 q1 │ q2
────┼────
(0 filas)

55432 13devel 17282=# fetch forward all in c1;
 q1 │ q2
────┼────
(0 filas)

So far so good .. things look normal.  But here's where things get
crazy:

55432 13devel 17282=# fetch backward all in c1;
        q1        │        q2       
──────────────────┼──────────────────
 4567890123456789 │              123
              123 │ 4567890123456789
(2 filas)

(huh???)

55432 13devel 17282=# fetch backward all in c1;
 q1 │ q2
────┼────
(0 filas)

Okay -- ignoring the fact that we got a row that should not be in the
result, we've exhausted the cursor going back.  Let's scan it again from
the start:

55432 13devel 17282=# fetch forward all in c1;
        q1        │        q2         
──────────────────┼───────────────────
              123 │  4567890123456789
 4567890123456789 │               123
 4567890123456789 │  4567890123456789
 4567890123456789 │ -4567890123456789
(4 filas)

This is just crazy.

I think you need to stare a that thing a little harder.


This is because the new state didn't know about backward scan
and there was off by one error it scan one position past to detect
ties. The attached patch fix both
regards
Surafel  
Attachment

pgsql-hackers by date:

Previous
From: Rafia Sabih
Date:
Subject: Re: How to prohibit parallel scan through tableam?
Next
From: Alvaro Herrera
Date:
Subject: Re: progress report for ANALYZE