Re: Implement for window functions - Mailing list pgsql-hackers

From Zhihong Yu
Subject Re: Implement for window functions
Date
Msg-id CALNJ-vQCZR29saSz5WOCkkktFP35NX+xQyusBAOdb3N_vVuDxA@mail.gmail.com
Whole thread Raw
In response to Re: Implement for window functions  (Krasiyan Andreev <krasiyan@gmail.com>)
Responses Re: Implement for window functions
List pgsql-hackers
Hi,
For WinGetFuncArgInFrame():

+   if (winobj->null_treatment == NULL_TREATMENT_IGNORE)
    {
...
+       if (target > 0)
+           step = 1;
+       else if (target < 0)
+           step = -1;
+       else if (seektype == WINDOW_SEEK_HEAD)
+           step = 1;
+       else if (seektype == WINDOW_SEEK_TAIL)
+           step = -1;
+       else
+           step = 0;
...
+       relpos = 0;
+   }

Why is relpos always set to 0 ?
In similar code in WinGetFuncArgInPartition(), I saw the following:

+           if (target > 0)
+               step = 1;
+           else if (target < 0)
+               step = -1;
+           else
+               step = 0;
+           relpos = step;

Maybe add a comment above the relpos assignment.

Thanks

On Sat, Jan 9, 2021 at 3:31 AM Krasiyan Andreev <krasiyan@gmail.com> wrote:
Hi, the building warning below is fixed now, no other changes. Also, I can confirm that the corner case with offset=0 in lead and lag works correctly.

gcc -Wall -Wmissing-prototypes -Wpointer-arith -Wdeclaration-after-statement -Werror=vla -Wendif-labels -Wmissing-format-attribute -Wimplicit-fallthrough=3 -Wcast-function-type -Wformat-security -fno-strict-aliasing -fwrapv -fexcess-precision=standard -Wno-format-truncation -Wno-stringop-truncation -O2 -I../../../src/include -I/home/krasiyan/pgsql/postgresql/src/include  -D_GNU_SOURCE -I/usr/include/libxml2   -c -o nodeWindowAgg.o /home/krasiyan/pgsql/postgresql/src/backend/executor/nodeWindowAgg.c
/home/krasiyan/pgsql/postgresql/src/backend/executor/nodeWindowAgg.c: In function ‘WinGetFuncArgInPartition’:
/home/krasiyan/pgsql/postgresql/src/backend/executor/nodeWindowAgg.c:3274:10: warning: ‘step’ may be used uninitialized in this function [-Wmaybe-uninitialized]
 3274 |   relpos += step;
      |   ~~~~~~~^~~~~~~
/home/krasiyan/pgsql/postgresql/src/backend/executor/nodeWindowAgg.c: In function ‘WinGetFuncArgInFrame’:
/home/krasiyan/pgsql/postgresql/src/backend/executor/nodeWindowAgg.c:3531:10: warning: ‘step’ may be used uninitialized in this function [-Wmaybe-uninitialized]
 3531 |   relpos += step;
      |   ~~~~~~~^~~~~~~



На пт, 8.01.2021 г. в 2:02 ч. Vik Fearing <vik@postgresfriends.org> написа:
On 1/1/21 10:21 PM, Zhihong Yu wrote:
> Krasiyan:
> Happy New Year.
>
> For WinGetFuncArgInPartition():
>
> +           if (target > 0)
> +               step = 1;
> +           else if (target < 0)
> +               step = -1;
> +           else
> +               step = 0;
>
> When would the last else statement execute ? Since the above code is
> for WINDOW_SEEK_CURRENT, I wonder why step should be 0.

Hi.

"lag(expr, 0) over w" is useless but valid.
--
Vik Fearing

pgsql-hackers by date:

Previous
From: Andrey Borodin
Date:
Subject: Re: Spurious "apparent wraparound" via SimpleLruTruncate() rounding
Next
From: Michael Banck
Date:
Subject: Fix \watch if expanded output is on and there are no results