Date: Thu, 28 Nov 2019 11:49:03 -0300
From: Alvaro Herrera <alvherre@2ndquadrant.com>
To: Surafel Temesgen <surafel3000@gmail.com>
Cc: Tomas Vondra <tomas.vondra@2ndquadrant.com>,
	PostgreSQL Hackers <pgsql-hackers@postgresql.org>,
	Andrew Gierth <andrew@tao11.riddles.org.uk>
Subject: Re: FETCH FIRST clause WITH TIES option
Message-ID: <20191128144903.GA30772@alvherre.pgsql>
MIME-Version: 1.0
Content-Type: text/plain; charset=iso-8859-1
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: 
 <CALAY4q82MDk1cehn-n2sSPu0iOLttXGk4A_CdXHnUSnQpK5sFA@mail.gmail.com>
User-Agent: Mutt/1.10.1 (2018-07-13)
Precedence: bulk

On 2019-Nov-28, Surafel Temesgen wrote:

> On Thu, Nov 28, 2019 at 12:36 AM Alvaro Herrera <alvherre@2ndquadrant.com>
> wrote:
> 
> > 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

But you did not read my code snippet, did you ...?

> > 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

Oh, thanks, glad it was that easy.

-- 
Álvaro Herrera                https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services