On 2/26/22 17:13, Chapman Flack wrote:
This applies (with some fuzz) and passes installcheck-world, but a rebase
is needed, because 3 lines of context aren't enough to get the doc changes
in the right place in the aggregate function table. (I think generating
the patch with 4 lines of context would be enough to keep that from being
a recurring issue.)
Thank you for the review and the tip re 4 lines of context! Rebase attached.
One thing that seems a bit funny is this message in the new
multirange_agg_transfn:
+ if (!type_is_multirange(mltrngtypoid))
+ ereport(ERROR,
+ (errcode(ERRCODE_DATATYPE_MISMATCH),
+ errmsg("range_agg must be called with a multirange")));
I agree it would be more helpful to users to let them know we can take
either kind of argument. I changed the message to "range_agg must be
called with a range or multirange". How does that seem?
I kind of wonder whether either message is really reachable, at least
through the aggregate machinery in the expected way. Won't that machinery
ensure that it is calling the right transfn with the right type of
argument? If that's the case, maybe the messages could only be seen
by someone calling the transfn directly ... which also seems ruled out
for these transfns because the state type is internal. Is this whole test
more of the nature of an assertion?
I don't think they are reachable, so perhaps they are more like asserts.
Do you think I should change it? It seems like a worthwhile check in any
case.
Yours,
--
Paul ~{:-)
p...@illuminatedcomputing.comFrom a6689485aab9b1aaa6e866f2a577368c7a0e324e 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 v2] Add range_agg with multirange inputs
---
doc/src/sgml/func.sgml | 30 ++++++
src/backend/utils/adt/multirangetypes.c | 62 ++++++++++-
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, 228 insertions(+), 1 deletion(-)
diff --git a/doc/src/sgml/func.sgml b/doc/src/sgml/func.sgml
index df3cd5987b..6a72785327 100644
--- a/doc/src/sgml/func.sgml
+++ b/doc/src/sgml/func.sgml
@@ -19959,8 +19959,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>max</primary>
@@ -20012,8 +20027,23 @@ 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>
diff --git a/src/backend/utils/adt/multirangetypes.c b/src/backend/utils/adt/multirangetypes.c
index c474b24431..0efef8cf35 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
@@ -1397,8 +1397,68 @@ 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))
+ ereport(ERROR,
+ (errcode(ERRCODE_DATATYPE_MISMATCH),
+ errmsg("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;
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..356fc7318b 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 => 'combine aggregate input into a multirange',
+ proname => 'range_agg', prokind => 'a', proisstrict => 'f',
+ prorettype => 'anymultirange', proargtypes => 'anymultirange',
+ prosrc => 'aggregate_dummy' },
+{ oid => '8002', descr => 'aggregate final function',
+ proname => 'multirange_agg_finalfn', proisstrict => 'f',
+ prorettype => 'anymultirange', proargtypes => 'internal anymultirange',
+ prosrc => 'range_agg_finalfn' },
{ 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
