Re: FETCH FIRST clause WITH TIES option

Поиск
Список
Период
Сортировка
От Surafel Temesgen
Тема Re: FETCH FIRST clause WITH TIES option
Дата
Msg-id CALAY4q82MDk1cehn-n2sSPu0iOLttXGk4A_CdXHnUSnQpK5sFA@mail.gmail.com
обсуждение исходный текст
Ответ на Re: FETCH FIRST clause WITH TIES option  (Alvaro Herrera <alvherre@2ndquadrant.com>)
Ответы Re: FETCH FIRST clause WITH TIES option  (Alvaro Herrera <alvherre@2ndquadrant.com>)
Список 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  
Вложения

В списке pgsql-hackers по дате отправления:

Предыдущее
От: Rafia Sabih
Дата:
Сообщение: Re: How to prohibit parallel scan through tableam?
Следующее
От: Alvaro Herrera
Дата:
Сообщение: Re: progress report for ANALYZE