From: | Zhihong Yu <zyu(at)yugabyte(dot)com> |
---|---|
To: | Krasiyan Andreev <krasiyan(at)gmail(dot)com> |
Cc: | Vik Fearing <vik(at)postgresfriends(dot)org>, David Fetter <david(at)fetter(dot)org>, PostgreSQL Hackers <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: Implement <null treatment> for window functions |
Date: | 2021-01-09 15:29:11 |
Message-ID: | CALNJ-vQCZR29saSz5WOCkkktFP35NX+xQyusBAOdb3N_vVuDxA@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | 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(at)gmail(dot)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(at)postgresfriends(dot)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
>>
>
From | Date | Subject | |
---|---|---|---|
Next Message | Michael Banck | 2021-01-09 15:40:59 | Fix \watch if expanded output is on and there are no results |
Previous Message | Andrey Borodin | 2021-01-09 15:25:39 | Re: Spurious "apparent wraparound" via SimpleLruTruncate() rounding |