You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@cloudberry.apache.org by le...@apache.org on 2026/09/09 08:50:11 UTC
(cloudberry) 01/02: Reject direct calls to the gp_percentile transition functions
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 940bab513d924495407d476b0031569ba046ee24
Author: Alena Rybakina <al...@gmail.com>
AuthorDate: Wed Aug 26 13:05:23 2026 +0300
Reject direct calls to the gp_percentile transition functions
gp_percentile_cont_{float8,interval,timestamp,timestamptz}_transition and
gp_percentile_disc_transition all read five arguments in C: the running
transition state plus the four arguments of the gp_percentile_cont() and
gp_percentile_disc() aggregates. pg_proc.dat, however, declares them with
four. As transition functions they are called correctly, since the executor
supplies state + 4 arguments regardless of the catalog, but a direct SQL call
reaches past the end of the argument array: PG_GETARG_INT64(4) picks up
garbage, which yields wrong results, an assertion when the bogus peer count
makes the code pfree() a NULL pointer, or a segfault.
Correcting the declaration would change the catalog and force an initdb, which
is not acceptable on a stable branch, so check the argument count instead and
raise a plain error. Direct calls were never useful - the functions only make
sense as the transition step of their aggregates - and 'percentile_* WITHIN
GROUP' queries are unaffected either way.
Co-authored-by: Georgy Shelkovy <g....@arenadata.io>
Ported from Greengage/open-gpdb commit 477b04a (ADBDEV-7770)
---
src/backend/utils/adt/orderedsetaggs.c | 22 ++++++++++++++++++++++
src/test/regress/expected/percentile.out | 24 ++++++++++++++++++++++++
src/test/regress/sql/percentile.sql | 14 ++++++++++++++
3 files changed, 60 insertions(+)
diff --git a/src/backend/utils/adt/orderedsetaggs.c b/src/backend/utils/adt/orderedsetaggs.c
index 46b2694d89e..efcadadce05 100644
--- a/src/backend/utils/adt/orderedsetaggs.c
+++ b/src/backend/utils/adt/orderedsetaggs.c
@@ -1516,6 +1516,17 @@ gp_percentile_cont_transition(FunctionCallInfo fcinfo,
int64 first_row;
int64 second_row;
+ /*
+ * Note: 'proargtypes' for this function in pg_proc.dat has 4 arguments.
+ * There are actually 5 arguments coming in here - the result of the
+ * previous call and 4 main arguments.
+ */
+ if (PG_NARGS() != 5)
+ ereport(ERROR,
+ (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+ errmsg("wrong number of arguments to gp_percentile_cont_transition()"),
+ errhint("expected 5, got %d", PG_NARGS())));
+
/* Return state for NULL inputs of val*/
if (PG_ARGISNULL(1) && !PG_ARGISNULL(0))
PG_RETURN_DATUM(PG_GETARG_DATUM(0));
@@ -1619,6 +1630,17 @@ gp_percentile_disc_transition(PG_FUNCTION_ARGS)
{
int64 rownum;
+ /*
+ * Note: 'proargtypes' for this function in pg_proc.dat has 4 arguments.
+ * There are actually 5 arguments coming in here - the result of the
+ * previous call and 4 main arguments.
+ */
+ if (PG_NARGS() != 5)
+ ereport(ERROR,
+ (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+ errmsg("wrong number of arguments to gp_percentile_disc_transition()"),
+ errhint("expected 5, got %d", PG_NARGS())));
+
/* Return state for NULL inputs of val*/
if (PG_ARGISNULL(1) && !PG_ARGISNULL(0))
PG_RETURN_DATUM(PG_GETARG_DATUM(0));
diff --git a/src/test/regress/expected/percentile.out b/src/test/regress/expected/percentile.out
index c1f4ec69da1..1589d50d8b6 100644
--- a/src/test/regress/expected/percentile.out
+++ b/src/test/regress/expected/percentile.out
@@ -876,6 +876,30 @@ group by d1, d2;
55 | 1
(1 row)
+--
+-- gp_percentile_cont()/gp_percentile_disc() are the split ordered-set
+-- aggregates that ORCA rewrites percentile_cont()/percentile_disc()/median()
+-- into. Their transition functions carry the running state on top of the
+-- four aggregate arguments, so they read five arguments, while pg_proc.dat
+-- describes four. That is left alone here so as not to force an initdb on a
+-- stable branch; instead a direct call, which would read past the end of the
+-- argument array, is rejected.
+--
+select gp_percentile_cont_float8_transition(NULL::float8, 1, 1, 1);
+ERROR: wrong number of arguments to gp_percentile_cont_transition()
+HINT: expected 5, got 4
+select gp_percentile_cont_interval_transition(NULL::interval, 1, 1, 1);
+ERROR: wrong number of arguments to gp_percentile_cont_transition()
+HINT: expected 5, got 4
+select gp_percentile_cont_timestamp_transition(NULL::timestamp, 1, 1, 1);
+ERROR: wrong number of arguments to gp_percentile_cont_transition()
+HINT: expected 5, got 4
+select gp_percentile_cont_timestamptz_transition(NULL::timestamptz, 1, 1, 1);
+ERROR: wrong number of arguments to gp_percentile_cont_transition()
+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
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 bc7c327e770..b7b622424e7 100644
--- a/src/test/regress/sql/percentile.sql
+++ b/src/test/regress/sql/percentile.sql
@@ -216,6 +216,20 @@ from mpp_22413
where d2 ='55'
group by d1, d2;
+--
+-- gp_percentile_cont()/gp_percentile_disc() are the split ordered-set
+-- aggregates that ORCA rewrites percentile_cont()/percentile_disc()/median()
+-- into. Their transition functions carry the running state on top of the
+-- four aggregate arguments, so they read five arguments, while pg_proc.dat
+-- describes four. That is left alone here so as not to force an initdb on a
+-- stable branch; instead a direct call, which would read past the end of the
+-- argument array, is rejected.
+--
+select gp_percentile_cont_float8_transition(NULL::float8, 1, 1, 1);
+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);
drop view percv2;
drop view percv;
drop table perct;
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@cloudberry.apache.org
For additional commands, e-mail: commits-help@cloudberry.apache.org