This is an automated email from the ASF dual-hosted git repository. leborchuk pushed a commit to branch REL_2_STABLE in repository https://gitbox.apache.org/repos/asf/cloudberry.git
commit 04035e768f8fc0123c7a56954027e245ce906208 Author: Alena Rybakina <[email protected]> AuthorDate: Wed Aug 26 13:06:01 2026 +0300 Keep the isnull flag when gp_percentile_* returns its previous state Both gp_percentile transition functions return the previous transition state untouched when the row they are looking at is not one of the rows the percentile is computed from. On the very first call that state is NULL, and returning it as a bare Datum(0) with isnull left false loses the flag: the aggregate yields 0 instead of NULL for by-value types, and dereferences a NULL pointer in the output function for by-reference ones - so an empty input set crashed the backend for the interval, timestamp and timestamptz variants. Co-authored-by: Georgy Shelkovy <[email protected]> Ported from Greengage/open-gpdb commit 477b04a (ADBDEV-7770). --- src/backend/utils/adt/orderedsetaggs.c | 21 +++++++--- src/test/regress/expected/percentile.out | 70 ++++++++++++++++++++++++++++++++ src/test/regress/sql/percentile.sql | 20 +++++++++ 3 files changed, 106 insertions(+), 5 deletions(-) diff --git a/src/backend/utils/adt/orderedsetaggs.c b/src/backend/utils/adt/orderedsetaggs.c index efcadadce05..32002dca684 100644 --- a/src/backend/utils/adt/orderedsetaggs.c +++ b/src/backend/utils/adt/orderedsetaggs.c @@ -1573,6 +1573,17 @@ gp_percentile_cont_transition(FunctionCallInfo fcinfo, { return_state = lerpfunc(prev_state, val, proportion); } + else if (PG_ARGISNULL(0)) + { + /* + * Neither of the rows we are after landed in this peer group, so we + * hand the previous state back unchanged. When that state is NULL + * the isnull flag has to travel with it: returning a bare Datum(0) + * as non-NULL gives wrong answers for by-value types and a NULL + * pointer dereference for by-reference ones. + */ + fcinfo->isnull = true; + } *cnt = *cnt + peer_count; if(*cnt > total_rows) @@ -1656,7 +1667,6 @@ gp_percentile_disc_transition(PG_FUNCTION_ARGS) errmsg("percentile value %g is not between 0 and 1", percentile))); Datum prev_state = PG_GETARG_DATUM(0); - bool prev_state_isnull = PG_ARGISNULL(0); Datum val = PG_GETARG_DATUM(1); Datum return_state = prev_state; int64 total_rows = PG_GETARG_INT64(3); @@ -1681,6 +1691,11 @@ gp_percentile_disc_transition(PG_FUNCTION_ARGS) { return_state = val; } + else if (PG_ARGISNULL(0)) + { + /* see gp_percentile_cont_transition() */ + fcinfo->isnull = true; + } *cnt = *cnt + peer_count; @@ -1691,10 +1706,6 @@ gp_percentile_disc_transition(PG_FUNCTION_ARGS) fcinfo->flinfo->fn_extra = NULL; } - if (return_state == prev_state) { - fcinfo->isnull = prev_state_isnull; - } - PG_RETURN_DATUM(return_state); } diff --git a/src/test/regress/expected/percentile.out b/src/test/regress/expected/percentile.out index 1589d50d8b6..563a4864995 100644 --- a/src/test/regress/expected/percentile.out +++ b/src/test/regress/expected/percentile.out @@ -900,6 +900,76 @@ HINT: expected 5, got 4 select gp_percentile_disc_transition(NULL::numeric, 1, 1, 1); ERROR: wrong number of arguments to gp_percentile_disc_transition() HINT: expected 5, got 4 +-- On an empty input set the transition state stays NULL. The transition +-- functions hand that state back untouched and have to keep its isnull flag +-- with it: a bare Datum(0) escaping here reads as a bogus value for by-value +-- types and dereferences a NULL pointer for by-reference ones. +select gp_percentile_cont(0::float8, 0, 0, 0); + gp_percentile_cont +-------------------- + +(1 row) + +select gp_percentile_cont('0 hour'::interval, 0, 0, 0); + gp_percentile_cont +-------------------- + +(1 row) + +select gp_percentile_cont('2006-01-01 13:10:13'::timestamp, 0, 0, 0); + gp_percentile_cont +-------------------- + +(1 row) + +select gp_percentile_cont('2006-01-01 13:10:13+00'::timestamptz, 0, 0, 0); + gp_percentile_cont +-------------------- + +(1 row) + +select gp_percentile_disc(0::numeric, 0, 0, 0); + gp_percentile_disc +-------------------- + +(1 row) + +-- A value that really was picked has to come back, even when its Datum +-- representation happens to be 0. +select gp_percentile_disc(0::float8, 0, 1, 1); + gp_percentile_disc +-------------------- + 0 +(1 row) + +select gp_percentile_disc(0::int, 0, 1, 1); + gp_percentile_disc +-------------------- + 0 +(1 row) + +select gp_percentile_cont(0::float8, 0, 1, 1); + gp_percentile_cont +-------------------- + 0 +(1 row) + +-- The same, end to end: the smallest value of b is 0. +create table perczero (a int, b float8) distributed by (a); +insert into perczero select i, (i - 1)::float8 from generate_series(1, 10) i; +select percentile_disc(0) within group (order by b) from perczero; + percentile_disc +----------------- + 0 +(1 row) + +select percentile_cont(0) within group (order by b) from perczero; + percentile_cont +----------------- + 0 +(1 row) + +drop table perczero; drop view percv2; drop view percv; drop table perct; diff --git a/src/test/regress/sql/percentile.sql b/src/test/regress/sql/percentile.sql index b7b622424e7..dde74fc29f5 100644 --- a/src/test/regress/sql/percentile.sql +++ b/src/test/regress/sql/percentile.sql @@ -230,6 +230,26 @@ select gp_percentile_cont_interval_transition(NULL::interval, 1, 1, 1); select gp_percentile_cont_timestamp_transition(NULL::timestamp, 1, 1, 1); select gp_percentile_cont_timestamptz_transition(NULL::timestamptz, 1, 1, 1); select gp_percentile_disc_transition(NULL::numeric, 1, 1, 1); +-- On an empty input set the transition state stays NULL. The transition +-- functions hand that state back untouched and have to keep its isnull flag +-- with it: a bare Datum(0) escaping here reads as a bogus value for by-value +-- types and dereferences a NULL pointer for by-reference ones. +select gp_percentile_cont(0::float8, 0, 0, 0); +select gp_percentile_cont('0 hour'::interval, 0, 0, 0); +select gp_percentile_cont('2006-01-01 13:10:13'::timestamp, 0, 0, 0); +select gp_percentile_cont('2006-01-01 13:10:13+00'::timestamptz, 0, 0, 0); +select gp_percentile_disc(0::numeric, 0, 0, 0); +-- A value that really was picked has to come back, even when its Datum +-- representation happens to be 0. +select gp_percentile_disc(0::float8, 0, 1, 1); +select gp_percentile_disc(0::int, 0, 1, 1); +select gp_percentile_cont(0::float8, 0, 1, 1); +-- The same, end to end: the smallest value of b is 0. +create table perczero (a int, b float8) distributed by (a); +insert into perczero select i, (i - 1)::float8 from generate_series(1, 10) i; +select percentile_disc(0) within group (order by b) from perczero; +select percentile_cont(0) within group (order by b) from perczero; +drop table perczero; drop view percv2; drop view percv; drop table perct; --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
