On 3/1/22 13:33, Chapman Flack wrote:
I think the 4 lines should suffice, but it looks like this patch was
generated from a rebase of the old one (with three lines) that ended up
putting the new 'range_agg' entry ahead of 'max' in func.sgml, which
position is now baked into the 4 lines of context. :)
You're right, my last rebase messed up the docs. Here it is fixed. Sorry
about that!
I would not change them to actual Assert, which would blow up the whole
process on failure. If it's a genuine "not expected to happen" case,
maybe changing it to elog (or ereport with errmsg_internal) would save
a little workload for translators.
I like the elog solution. I've changed them in both places.
I did a small double-take seeing the C range_agg_finalfn being shared
by the SQL range_agg_finalfn and multirange_agg_finalfn. I infer that
the reason it works is get_fn_expr_rettype works equally well with
either parameter type.
Do you think it would be worth adding a comment at the C function
explaining that? In a quick query I just did, I found no other aggregate
final functions sharing a C function that way, so this could be the first.
I see 13 other shared finalfns (using select
array_agg(aggfnoid::regproc) as procs, array_agg(aggtransfn) as
transfns, aggfinalfn from pg_aggregate where aggfinalfn is distinct from
0 group by aggfinalfn having count(*) > 1;) but a comment can't hurt! Added.
Thanks,
--
Paul ~{:-)
p...@illuminatedcomputing.comFrom 1e7a96668b0ad341d29281055927ad693c656843 Mon Sep 17 00:00:00 2001
From: "Paul A. Jungwirth" <p...@illuminatedcomputing.com>
Date: Fri, 10 Dec 2021 16:04:57 -0800
Subject: [PATCH v3] Add range_agg with multirange inputs
---
doc/src/sgml/func.sgml | 45 ++++++++
src/backend/utils/adt/multirangetypes.c | 66 +++++++++++-
src/include/catalog/pg_aggregate.dat | 3 +
src/include/catalog/pg_proc.dat | 11 ++
src/test/regress/expected/multirangetypes.out | 100 ++++++++++++++++++
src/test/regress/expected/opr_sanity.out | 1 +
src/test/regress/sql/multirangetypes.sql | 21 ++++
src/test/regress/sql/opr_sanity.sql | 1 +
8 files changed, 244 insertions(+), 4 deletions(-)
diff --git a/doc/src/sgml/func.sgml b/doc/src/sgml/func.sgml
index df3cd5987b..fb1963a687 100644
--- a/doc/src/sgml/func.sgml
+++ b/doc/src/sgml/func.sgml
@@ -20012,8 +20012,23 @@ SELECT NULLIF(value, '(none)') ...
</para></entry>
<entry>No</entry>
</row>
+ <row>
+ <entry role="func_table_entry"><para role="func_signature">
+ <indexterm>
+ <primary>range_agg</primary>
+ </indexterm>
+ <function>range_agg</function> ( <parameter>value</parameter>
+ <type>anymultirange</type> )
+ <returnvalue>anymultirange</returnvalue>
+ </para>
+ <para>
+ Computes the union of the non-null input values.
+ </para></entry>
+ <entry>No</entry>
+ </row>
+
<row>
<entry role="func_table_entry"><para role="func_signature">
<indexterm>
<primary>range_intersect_agg</primary>
@@ -20027,8 +20042,38 @@ SELECT NULLIF(value, '(none)') ...
</para></entry>
<entry>No</entry>
</row>
+ <row>
+ <entry role="func_table_entry"><para role="func_signature">
+ <indexterm>
+ <primary>range_intersect_agg</primary>
+ </indexterm>
+ <function>range_intersect_agg</function> ( <parameter>value</parameter>
+ <type>anymultirange</type> )
+ <returnvalue>anymultirange</returnvalue>
+ </para>
+ <para>
+ Computes the intersection of the non-null input values.
+ </para></entry>
+ <entry>No</entry>
+ </row>
+
+ <row>
+ <entry role="func_table_entry"><para role="func_signature">
+ <indexterm>
+ <primary>range_intersect_agg</primary>
+ </indexterm>
+ <function>range_intersect_agg</function> ( <parameter>value</parameter>
+ <type>anymultirange</type> )
+ <returnvalue>anymultirange</returnvalue>
+ </para>
+ <para>
+ Computes the intersection of the non-null input values.
+ </para></entry>
+ <entry>No</entry>
+ </row>
+
<row>
<entry role="func_table_entry"><para role="func_signature">
<indexterm>
<primary>string_agg</primary>
diff --git a/src/backend/utils/adt/multirangetypes.c b/src/backend/utils/adt/multirangetypes.c
index c474b24431..5a4c00457a 100644
--- a/src/backend/utils/adt/multirangetypes.c
+++ b/src/backend/utils/adt/multirangetypes.c
@@ -1346,9 +1346,9 @@ range_agg_transfn(PG_FUNCTION_ARGS)
rngtypoid = get_fn_expr_argtype(fcinfo->flinfo, 1);
if (!type_is_range(rngtypoid))
ereport(ERROR,
(errcode(ERRCODE_DATATYPE_MISMATCH),
- errmsg("range_agg must be called with a range")));
+ errmsg("range_agg must be called with a range or multirange")));
if (PG_ARGISNULL(0))
state = initArrayResult(rngtypoid, aggContext, false);
else
@@ -1362,8 +1362,10 @@ range_agg_transfn(PG_FUNCTION_ARGS)
}
/*
* range_agg_finalfn: use our internal array to merge touching ranges.
+ *
+ * Shared by range_agg(anyrange) and range_agg(anymultirange).
*/
Datum
range_agg_finalfn(PG_FUNCTION_ARGS)
{
@@ -1397,8 +1399,66 @@ range_agg_finalfn(PG_FUNCTION_ARGS)
PG_RETURN_MULTIRANGE_P(make_multirange(mltrngtypoid, typcache->rngtype, range_count, ranges));
}
+/*
+ * multirange_agg_transfn: combine adjacent/overlapping multiranges.
+ *
+ * All we do here is gather the input multiranges' ranges into an array
+ * so that the finalfn can sort and combine them.
+ */
+Datum
+multirange_agg_transfn(PG_FUNCTION_ARGS)
+{
+ MemoryContext aggContext;
+ Oid mltrngtypoid;
+ TypeCacheEntry *typcache;
+ TypeCacheEntry *rngtypcache;
+ ArrayBuildState *state;
+ MultirangeType *current;
+ int32 range_count;
+ RangeType **ranges;
+ int32 i;
+
+ if (!AggCheckCallContext(fcinfo, &aggContext))
+ elog(ERROR, "multirange_agg_transfn called in non-aggregate context");
+
+ mltrngtypoid = get_fn_expr_argtype(fcinfo->flinfo, 1);
+ if (!type_is_multirange(mltrngtypoid))
+ elog(ERROR, "range_agg must be called with a range or multirange");
+
+ typcache = multirange_get_typcache(fcinfo, mltrngtypoid);
+ rngtypcache = typcache->rngtype;
+
+ if (PG_ARGISNULL(0))
+ state = initArrayResult(rngtypcache->type_id, aggContext, false);
+ else
+ state = (ArrayBuildState *) PG_GETARG_POINTER(0);
+
+ /* skip NULLs */
+ if (!PG_ARGISNULL(1))
+ {
+ current = PG_GETARG_MULTIRANGE_P(1);
+ multirange_deserialize(rngtypcache, current, &range_count, &ranges);
+ if (range_count == 0)
+ {
+ /*
+ * Add an empty range so we get an empty result (not a null result).
+ */
+ accumArrayResult(state,
+ RangeTypePGetDatum(make_empty_range(rngtypcache)),
+ false, rngtypcache->type_id, aggContext);
+ }
+ else
+ {
+ for (i = 0; i < range_count; i++)
+ accumArrayResult(state, RangeTypePGetDatum(ranges[i]), false, rngtypcache->type_id, aggContext);
+ }
+ }
+
+ PG_RETURN_POINTER(state);
+}
+
Datum
multirange_intersect_agg_transfn(PG_FUNCTION_ARGS)
{
MemoryContext aggContext;
@@ -1415,11 +1475,9 @@ multirange_intersect_agg_transfn(PG_FUNCTION_ARGS)
elog(ERROR, "multirange_intersect_agg_transfn called in non-aggregate context");
mltrngtypoid = get_fn_expr_argtype(fcinfo->flinfo, 1);
if (!type_is_multirange(mltrngtypoid))
- ereport(ERROR,
- (errcode(ERRCODE_DATATYPE_MISMATCH),
- errmsg("range_intersect_agg must be called with a multirange")));
+ elog(ERROR, "range_agg must be called with a range or multirange");
typcache = multirange_get_typcache(fcinfo, mltrngtypoid);
/* strictness ensures these are non-null */
diff --git a/src/include/catalog/pg_aggregate.dat b/src/include/catalog/pg_aggregate.dat
index 2843f4b415..15b4e6461a 100644
--- a/src/include/catalog/pg_aggregate.dat
+++ b/src/include/catalog/pg_aggregate.dat
@@ -562,8 +562,11 @@
aggtranstype => 'anymultirange' },
{ aggfnoid => 'range_agg(anyrange)', aggtransfn => 'range_agg_transfn',
aggfinalfn => 'range_agg_finalfn', aggfinalextra => 't',
aggtranstype => 'internal' },
+{ aggfnoid => 'range_agg(anymultirange)', aggtransfn => 'multirange_agg_transfn',
+ aggfinalfn => 'multirange_agg_finalfn', aggfinalextra => 't',
+ aggtranstype => 'internal' },
# json
{ aggfnoid => 'json_agg', aggtransfn => 'json_agg_transfn',
aggfinalfn => 'json_agg_finalfn', aggtranstype => 'internal' },
diff --git a/src/include/catalog/pg_proc.dat b/src/include/catalog/pg_proc.dat
index bf88858171..1f0da00a26 100644
--- a/src/include/catalog/pg_proc.dat
+++ b/src/include/catalog/pg_proc.dat
@@ -10611,8 +10611,19 @@
{ oid => '4301', descr => 'combine aggregate input into a multirange',
proname => 'range_agg', prokind => 'a', proisstrict => 'f',
prorettype => 'anymultirange', proargtypes => 'anyrange',
prosrc => 'aggregate_dummy' },
+{ oid => '8000', descr => 'aggregate transition function',
+ proname => 'multirange_agg_transfn', proisstrict => 'f', prorettype => 'internal',
+ proargtypes => 'internal anymultirange', prosrc => 'multirange_agg_transfn' },
+{ oid => '8001', descr => 'aggregate final function',
+ proname => 'multirange_agg_finalfn', proisstrict => 'f',
+ prorettype => 'anymultirange', proargtypes => 'internal anymultirange',
+ prosrc => 'range_agg_finalfn' },
+{ oid => '8002', descr => 'combine aggregate input into a multirange',
+ proname => 'range_agg', prokind => 'a', proisstrict => 'f',
+ prorettype => 'anymultirange', proargtypes => 'anymultirange',
+ prosrc => 'aggregate_dummy' },
{ oid => '4388', descr => 'range aggregate by intersecting',
proname => 'multirange_intersect_agg_transfn', prorettype => 'anymultirange',
proargtypes => 'anymultirange anymultirange',
prosrc => 'multirange_intersect_agg_transfn' },
diff --git a/src/test/regress/expected/multirangetypes.out b/src/test/regress/expected/multirangetypes.out
index bde3f248a7..ac2eb84c3a 100644
--- a/src/test/regress/expected/multirangetypes.out
+++ b/src/test/regress/expected/multirangetypes.out
@@ -2783,8 +2783,72 @@ FROM (VALUES
---------------
{[a,f],[g,j)}
(1 row)
+-- range_agg with multirange inputs
+select range_agg(nmr) from nummultirange_test;
+ range_agg
+-----------
+ {(,)}
+(1 row)
+
+select range_agg(nmr) from nummultirange_test where false;
+ range_agg
+-----------
+
+(1 row)
+
+select range_agg(null::nummultirange) from nummultirange_test;
+ range_agg
+-----------
+
+(1 row)
+
+select range_agg(nmr) from (values ('{}'::nummultirange)) t(nmr);
+ range_agg
+-----------
+ {}
+(1 row)
+
+select range_agg(nmr) from (values ('{}'::nummultirange), ('{}'::nummultirange)) t(nmr);
+ range_agg
+-----------
+ {}
+(1 row)
+
+select range_agg(nmr) from (values ('{[1,2]}'::nummultirange)) t(nmr);
+ range_agg
+-----------
+ {[1,2]}
+(1 row)
+
+select range_agg(nmr) from (values ('{[1,2], [5,6]}'::nummultirange)) t(nmr);
+ range_agg
+---------------
+ {[1,2],[5,6]}
+(1 row)
+
+select range_agg(nmr) from (values ('{[1,2], [2,3]}'::nummultirange)) t(nmr);
+ range_agg
+-----------
+ {[1,3]}
+(1 row)
+
+select range_agg(nmr) from (values ('{[1,2]}'::nummultirange), ('{[5,6]}'::nummultirange)) t(nmr);
+ range_agg
+---------------
+ {[1,2],[5,6]}
+(1 row)
+
+select range_agg(nmr) from (values ('{[1,2]}'::nummultirange), ('{[2,3]}'::nummultirange)) t(nmr);
+ range_agg
+-----------
+ {[1,3]}
+(1 row)
+
+--
+-- range_intersect_agg function
+--
select range_intersect_agg(nmr) from nummultirange_test;
range_intersect_agg
---------------------
{}
@@ -2795,15 +2859,51 @@ select range_intersect_agg(nmr) from nummultirange_test where false;
---------------------
(1 row)
+select range_intersect_agg(null::nummultirange) from nummultirange_test;
+ range_intersect_agg
+---------------------
+
+(1 row)
+
+select range_intersect_agg(nmr) from (values ('{[1,3]}'::nummultirange), ('{[6,12]}'::nummultirange)) t(nmr);
+ range_intersect_agg
+---------------------
+ {}
+(1 row)
+
+select range_intersect_agg(nmr) from (values ('{[1,6]}'::nummultirange), ('{[3,12]}'::nummultirange)) t(nmr);
+ range_intersect_agg
+---------------------
+ {[3,6]}
+(1 row)
+
+select range_intersect_agg(nmr) from (values ('{[1,6], [10,12]}'::nummultirange), ('{[4,14]}'::nummultirange)) t(nmr);
+ range_intersect_agg
+---------------------
+ {[4,6],[10,12]}
+(1 row)
+
-- test with just one input:
+select range_intersect_agg(nmr) from (values ('{}'::nummultirange)) t(nmr);
+ range_intersect_agg
+---------------------
+ {}
+(1 row)
+
select range_intersect_agg(nmr) from (values ('{[1,2]}'::nummultirange)) t(nmr);
range_intersect_agg
---------------------
{[1,2]}
(1 row)
+select range_intersect_agg(nmr) from (values ('{[1,6], [10,12]}'::nummultirange)) t(nmr);
+ range_intersect_agg
+---------------------
+ {[1,6],[10,12]}
+(1 row)
+
select range_intersect_agg(nmr) from nummultirange_test where nmr @> 4.0;
range_intersect_agg
---------------------
{[3,5)}
diff --git a/src/test/regress/expected/opr_sanity.out b/src/test/regress/expected/opr_sanity.out
index 4ce6c039b4..046d2006d5 100644
--- a/src/test/regress/expected/opr_sanity.out
+++ b/src/test/regress/expected/opr_sanity.out
@@ -192,8 +192,9 @@ WHERE p1.oid != p2.oid AND
p1.prosrc NOT LIKE E'range\\_constructor_' AND
p2.prosrc NOT LIKE E'range\\_constructor_' AND
p1.prosrc NOT LIKE E'multirange\\_constructor_' AND
p2.prosrc NOT LIKE E'multirange\\_constructor_' AND
+ p2.prosrc NOT LIKE E'range\\_agg\\_finalfn' AND
(p1.proargtypes[1] < p2.proargtypes[1])
ORDER BY 1, 2;
proargtypes | proargtypes
-----------------------------+--------------------------
diff --git a/src/test/regress/sql/multirangetypes.sql b/src/test/regress/sql/multirangetypes.sql
index df1edb4d31..1abcaeddb5 100644
--- a/src/test/regress/sql/multirangetypes.sql
+++ b/src/test/regress/sql/multirangetypes.sql
@@ -571,12 +571,33 @@ FROM (VALUES
('[g,h)'::textrange),
('[h,j)'::textrange)
) t(r);
+-- range_agg with multirange inputs
+select range_agg(nmr) from nummultirange_test;
+select range_agg(nmr) from nummultirange_test where false;
+select range_agg(null::nummultirange) from nummultirange_test;
+select range_agg(nmr) from (values ('{}'::nummultirange)) t(nmr);
+select range_agg(nmr) from (values ('{}'::nummultirange), ('{}'::nummultirange)) t(nmr);
+select range_agg(nmr) from (values ('{[1,2]}'::nummultirange)) t(nmr);
+select range_agg(nmr) from (values ('{[1,2], [5,6]}'::nummultirange)) t(nmr);
+select range_agg(nmr) from (values ('{[1,2], [2,3]}'::nummultirange)) t(nmr);
+select range_agg(nmr) from (values ('{[1,2]}'::nummultirange), ('{[5,6]}'::nummultirange)) t(nmr);
+select range_agg(nmr) from (values ('{[1,2]}'::nummultirange), ('{[2,3]}'::nummultirange)) t(nmr);
+
+--
+-- range_intersect_agg function
+--
select range_intersect_agg(nmr) from nummultirange_test;
select range_intersect_agg(nmr) from nummultirange_test where false;
+select range_intersect_agg(null::nummultirange) from nummultirange_test;
+select range_intersect_agg(nmr) from (values ('{[1,3]}'::nummultirange), ('{[6,12]}'::nummultirange)) t(nmr);
+select range_intersect_agg(nmr) from (values ('{[1,6]}'::nummultirange), ('{[3,12]}'::nummultirange)) t(nmr);
+select range_intersect_agg(nmr) from (values ('{[1,6], [10,12]}'::nummultirange), ('{[4,14]}'::nummultirange)) t(nmr);
-- test with just one input:
+select range_intersect_agg(nmr) from (values ('{}'::nummultirange)) t(nmr);
select range_intersect_agg(nmr) from (values ('{[1,2]}'::nummultirange)) t(nmr);
+select range_intersect_agg(nmr) from (values ('{[1,6], [10,12]}'::nummultirange)) t(nmr);
select range_intersect_agg(nmr) from nummultirange_test where nmr @> 4.0;
create table nummultirange_test2(nmr nummultirange);
create index nummultirange_test2_hash_idx on nummultirange_test2 using hash (nmr);
diff --git a/src/test/regress/sql/opr_sanity.sql b/src/test/regress/sql/opr_sanity.sql
index 2b292851e3..bbcffda228 100644
--- a/src/test/regress/sql/opr_sanity.sql
+++ b/src/test/regress/sql/opr_sanity.sql
@@ -152,8 +152,9 @@ WHERE p1.oid != p2.oid AND
p1.prosrc NOT LIKE E'range\\_constructor_' AND
p2.prosrc NOT LIKE E'range\\_constructor_' AND
p1.prosrc NOT LIKE E'multirange\\_constructor_' AND
p2.prosrc NOT LIKE E'multirange\\_constructor_' AND
+ p2.prosrc NOT LIKE E'range\\_agg\\_finalfn' AND
(p1.proargtypes[1] < p2.proargtypes[1])
ORDER BY 1, 2;
SELECT DISTINCT p1.proargtypes[2]::regtype, p2.proargtypes[2]::regtype
--
2.25.1
