This is an automated email from the ASF dual-hosted git repository.
jgemignani pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/incubator-age.git
The following commit(s) were added to refs/heads/master by this push:
new e36983b Fix bug in aggregate function collect()
e36983b is described below
commit e36983b21d966c0628959ceb6633339e79062c74
Author: John Gemignani <[email protected]>
AuthorDate: Tue May 10 12:03:44 2022 -0700
Fix bug in aggregate function collect()
Fixes a bug where the collect functions aggtransfn is not called but
the aggfinalfn is. While I'm not sure why this case happens, as it
'should' always call the aggtransfn, it crashes because the collect
aggregate state (castate) hasn't been initialized and is still null.
The fix is to have the aggfinalfn check for a null state and create
the state for finalizing the empty '[]' results.
Added a regression test for this case.
---
regress/expected/expr.out | 6 ++++++
regress/sql/expr.sql | 2 ++
src/backend/utils/adt/agtype.c | 31 +++++++++++++++++++++++++++++--
3 files changed, 37 insertions(+), 2 deletions(-)
diff --git a/regress/expected/expr.out b/regress/expected/expr.out
index 5c8df1f..0bcdf6a 100644
--- a/regress/expected/expr.out
+++ b/regress/expected/expr.out
@@ -5224,6 +5224,12 @@ SELECT * FROM cypher('UCSC', $$ RETURN collect(NULL) $$)
AS (empty agtype);
[]
(1 row)
+SELECT * FROM cypher('UCSC', $$ MATCH (u) WHERE u.name =~ "doesn't exist"
RETURN collect(u.name) $$) AS (name agtype);
+ name
+------
+ []
+(1 row)
+
-- should fail
SELECT * FROM cypher('UCSC', $$ RETURN collect() $$) AS (collect agtype);
ERROR: function ag_catalog.age_collect() does not exist
diff --git a/regress/sql/expr.sql b/regress/sql/expr.sql
index 745388d..b653575 100644
--- a/regress/sql/expr.sql
+++ b/regress/sql/expr.sql
@@ -2182,6 +2182,8 @@ AS (zip1 agtype, zip2 agtype);
SELECT * FROM cypher('UCSC', $$ RETURN collect(5) $$) AS (result agtype);
-- should return an empty aray
SELECT * FROM cypher('UCSC', $$ RETURN collect(NULL) $$) AS (empty agtype);
+SELECT * FROM cypher('UCSC', $$ MATCH (u) WHERE u.name =~ "doesn't exist"
RETURN collect(u.name) $$) AS (name agtype);
+
-- should fail
SELECT * FROM cypher('UCSC', $$ RETURN collect() $$) AS (collect agtype);
diff --git a/src/backend/utils/adt/agtype.c b/src/backend/utils/adt/agtype.c
index 1c644aa..be52047 100644
--- a/src/backend/utils/adt/agtype.c
+++ b/src/backend/utils/adt/agtype.c
@@ -8933,7 +8933,9 @@ Datum age_collect_aggtransfn(PG_FUNCTION_ARGS)
}
/* otherwise, retrieve the state */
else
+ {
castate = (agtype_in_state *) PG_GETARG_POINTER(0);
+ }
/*
* Extract the variadic args, of which there should only be one.
@@ -8941,9 +8943,13 @@ Datum age_collect_aggtransfn(PG_FUNCTION_ARGS)
* skipped over.
*/
if (PG_ARGISNULL(1))
+ {
nargs = 0;
+ }
else
+ {
nargs = extract_variadic_args(fcinfo, 1, true, &args, &types, &nulls);
+ }
if (nargs == 1)
{
@@ -8962,15 +8968,21 @@ Datum age_collect_aggtransfn(PG_FUNCTION_ARGS)
0);
/* add the arg if not agtype null */
if (agtv_value->type != AGTV_NULL)
+ {
add_agtype(args[0], nulls[0], castate, types[0], false);
+ }
}
else
+ {
add_agtype(args[0], nulls[0], castate, types[0], false);
+ }
}
}
else if (nargs > 1)
+ {
ereport(ERROR, (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
errmsg("collect() invalid number of arguments")));
+ }
/* restore the old context */
MemoryContextSwitchTo(old_mcxt);
@@ -8988,8 +9000,23 @@ Datum age_collect_aggfinalfn(PG_FUNCTION_ARGS)
/* verify we are in an aggregate context */
Assert(AggCheckCallContext(fcinfo, NULL) == AGG_CONTEXT_AGGREGATE);
- /* get the state */
- castate = (agtype_in_state *) PG_GETARG_POINTER(0);
+ /*
+ * Get the state. There are cases where the age_collect_aggtransfn never
+ * gets called. So, check to see if this is one.
+ */
+ if (PG_ARGISNULL(0))
+ {
+ /* create and initialize the state */
+ castate = palloc0(sizeof(agtype_in_state));
+ memset(castate, 0, sizeof(agtype_in_state));
+ /* start the array */
+ castate->res = push_agtype_value(&castate->parse_state,
+ WAGT_BEGIN_ARRAY, NULL);
+ }
+ else
+ {
+ castate = (agtype_in_state *) PG_GETARG_POINTER(0);
+ }
/* switch to the correct aggregate context */
old_mcxt = MemoryContextSwitchTo(fcinfo->flinfo->fn_mcxt);
/* Finish/close the array */
