Hi Jian, > I confirmed the problem. It seems it's a corner case bug: only happens > when offset = -2147483648. Anyway, bug is a bug. > > Attached is a patch which should be ready for commit IMO. I haven't > changed the C code part from yours. I have changed the regression test > part to use low case SQL statements which fits in better with > surroundings.
Attached is the v2 patch. I just added a test case for lag() to clarify that it is affected by the bug too. If there's no objection, I will push it to master and 19 branches. Regards, -- Tatsuo Ishii SRA OSS K.K. English: http://www.sraoss.co.jp/index_en/ Japanese:http://www.sraoss.co.jp
>From 6d9071bfb5c1f1aa884fc0ef672691220a0fcaf8 Mon Sep 17 00:00:00 2001 From: Tatsuo Ishii <[email protected]> Date: Thu, 5 Nov 2026 12:26:42 +0900 Subject: [PATCH v2] Fix wrong results of some window functions with IGNORE NULLS. Window functions such as lead() and lag() could return wrong results if IGNORE NULLS is specified and the offset argument is equal to INT_MIN. WinGetFuncArgInPartition() used abs() to get the absolute value of the offset argument. If it is INT_MIN (-2147483648), the return value of abs() is undefined. Typically it returns INT_MIN. However, WinGetFuncArgInPartition() was not prepared for the case and returns wrong result. To fix this, use pg_abs_s32() instead of abs() to return 2147483648, instead of -2147483648. ignorenulls_getfuncarginframe() has the same problem. So this commit fixes them as well. Note that currently no built in window functions hits the bug because only nth_value() has the offset argument but it disallows to accept negative offset. However, as WinGetFuncArgInFrame(), the caller of ignorenulls_getfuncarginframe(), is a public API, we should prepare the similar case when user defined window functions use it. Moreover, this commit changes some internal variable data types from int to int64 in order to prevent an overflow while calculating a row position in the partition. Also add a regression test case added to window.sql. Author: jian he <[email protected]> Reviewed-by: Tatsuo Ishii <[email protected]> Discussion: https://postgr.es/m/CACJufxFnm%2BAj2Jyhyd58PtW8e1vTZDKimkZE%2BMashCPSDKw56Q%40mail.gmail.com Backpatch-through: 19 --- src/backend/executor/nodeWindowAgg.c | 12 ++++++------ src/test/regress/expected/window.out | 22 ++++++++++++++++++++++ src/test/regress/sql/window.sql | 12 ++++++++++++ 3 files changed, 40 insertions(+), 6 deletions(-) diff --git a/src/backend/executor/nodeWindowAgg.c b/src/backend/executor/nodeWindowAgg.c index b86dcbba055..d85dd4978dd 100644 --- a/src/backend/executor/nodeWindowAgg.c +++ b/src/backend/executor/nodeWindowAgg.c @@ -3415,8 +3415,8 @@ ignorenulls_getfuncarginframe(WindowObject winobj, int argno, Datum datum; int64 abs_pos; int64 mark_pos; - int notnull_offset; - int notnull_relpos; + int64 notnull_offset; + int64 notnull_relpos; int forward; Assert(WindowObjectIsValid(winobj)); @@ -3425,7 +3425,7 @@ ignorenulls_getfuncarginframe(WindowObject winobj, int argno, slot = winstate->temp_slot_1; datum = (Datum) 0; notnull_offset = 0; - notnull_relpos = abs(relpos); + notnull_relpos = pg_abs_s32(relpos); switch (seektype) { @@ -3859,8 +3859,8 @@ WinGetFuncArgInPartition(WindowObject winobj, int argno, int64 mark_pos; Datum datum; bool null_treatment; - int notnull_offset; - int notnull_relpos; + int64 notnull_offset; + int64 notnull_relpos; int forward; bool myisout; bool got_datum; @@ -3909,7 +3909,7 @@ WinGetFuncArgInPartition(WindowObject winobj, int argno, /* Prepare for loop */ notnull_offset = 0; - notnull_relpos = abs(relpos); + notnull_relpos = pg_abs_s32(relpos); forward = relpos > 0 ? 1 : -1; myisout = false; got_datum = false; diff --git a/src/test/regress/expected/window.out b/src/test/regress/expected/window.out index c0bde1c5eec..7d4b3d51365 100644 --- a/src/test/regress/expected/window.out +++ b/src/test/regress/expected/window.out @@ -2143,6 +2143,28 @@ from generate_series(-9223372036854775806, -9223372036854775804) x; -9223372036854775806 | -9223372036854775806 (3 rows) +SELECT x, + lead(x, -2147483648) over w as respect_nulls, + lead(x, -2147483648) ignore nulls over w as ignore_nulls +from (values (1), (2)) as t(x) +window w as (order by x); + x | respect_nulls | ignore_nulls +---+---------------+-------------- + 1 | | + 2 | | +(2 rows) + +SELECT x, + lag(x, -2147483648) over w as respect_nulls, + lag(x, -2147483648) ignore nulls over w as ignore_nulls +from (values (1), (2)) as t(x) +window w as (order by x); + x | respect_nulls | ignore_nulls +---+---------------+-------------- + 1 | | + 2 | | +(2 rows) + -- Test in_range for other numeric datatypes create temp table numerics( id int, diff --git a/src/test/regress/sql/window.sql b/src/test/regress/sql/window.sql index 8e6f92d94c7..dc5294f2f9f 100644 --- a/src/test/regress/sql/window.sql +++ b/src/test/regress/sql/window.sql @@ -587,6 +587,18 @@ from generate_series(9223372036854775804, 9223372036854775806) x; select x, last_value(x) over (order by x desc range between current row and 5 following) from generate_series(-9223372036854775806, -9223372036854775804) x; +SELECT x, + lead(x, -2147483648) over w as respect_nulls, + lead(x, -2147483648) ignore nulls over w as ignore_nulls +from (values (1), (2)) as t(x) +window w as (order by x); + +SELECT x, + lag(x, -2147483648) over w as respect_nulls, + lag(x, -2147483648) ignore nulls over w as ignore_nulls +from (values (1), (2)) as t(x) +window w as (order by x); + -- Test in_range for other numeric datatypes create temp table numerics( -- 2.43.0
