Hi Jian, >> SELECT v, >> lead(v, -2147483648) OVER w AS respect_nulls, >> lead(v, -2147483648) IGNORE NULLS OVER w AS ignore_nulls >> FROM (VALUES (1), (2)) AS t(v) >> WINDOW w AS (ORDER BY v); >> >> should return all NULL values for column ignore_nulls. >> >> The fix is to replace the abs function in nodeWindowAgg.c with pg_abs_s32.
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. Regards, -- Tatsuo Ishii SRA OSS K.K. English: http://www.sraoss.co.jp/index_en/ Japanese:http://www.sraoss.co.jp
>From d26507fb9752a77e198d9869d401ad189125ac9e Mon Sep 17 00:00:00 2001 From: Tatsuo Ishii <[email protected]> Date: Wed, 4 Nov 2026 16:15:50 +0900 Subject: [PATCH v1] Fix some window functions return wrong results with IGNORE NULLS. Window functions such as lead() and lag() returns 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 | 11 +++++++++++ src/test/regress/sql/window.sql | 6 ++++++ 3 files changed, 23 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..0a07fd28ec4 100644 --- a/src/test/regress/expected/window.out +++ b/src/test/regress/expected/window.out @@ -2143,6 +2143,17 @@ 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) + -- 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..419e3aa6dfc 100644 --- a/src/test/regress/sql/window.sql +++ b/src/test/regress/sql/window.sql @@ -587,6 +587,12 @@ 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); + -- Test in_range for other numeric datatypes create temp table numerics( -- 2.43.0
