Re: range_agg - Mailing list pgsql-hackers
| From | Pavel Stehule |
|---|---|
| Subject | Re: range_agg |
| Date | |
| Msg-id | CAFj8pRBZJ6aiLXOf7SDT=R4pscN--44YmGAaeP_qSr9eaS1MCA@mail.gmail.com Whole thread Raw |
| In response to | Re: range_agg (Alvaro Herrera <alvherre@2ndquadrant.com>) |
| Responses |
Re: range_agg
(Paul A Jungwirth <pj@illuminatedcomputing.com>)
|
| List | pgsql-hackers |
čt 4. 7. 2019 v 20:34 odesílatel Alvaro Herrera <alvherre@2ndquadrant.com> napsal:
I noticed that this patch has a // comment about it segfaulting. Did
you ever figure that out? Is the resulting code the one you intend as
final?
Did you make any inroads regarding Jeff Davis' suggestion about
implementing "multiranges"? I wonder if that's going to end up being a
Pandora's box.
Introduction of new datatype can be better, because we can ensure so data are correctly ordered
Stylistically, the code does not match pgindent's choices very closely.
I can return a diff to a pgindented version of your v0002 for your
perusal, if it would be useful for you to learn its style. (A committer
would eventually run pgindent himself[1], but it's good to have
submissions be at least close to the final form.) BTW, I suggest "git
format-patch -vN" to prepare files for submission.
The first issue is unstable regress tests - there is a problem with opr_sanity
SELECT p1.oid, p1.proname, p2.oid, p2.proname
FROM pg_proc AS p1, pg_proc AS p2
WHERE p1.oid < p2.oid AND
p1.prosrc = p2.prosrc AND
p1.prolang = 12 AND p2.prolang = 12 AND
(p1.prokind != 'a' OR p2.prokind != 'a') AND
(p1.prolang != p2.prolang OR
p1.prokind != p2.prokind OR
p1.prosecdef != p2.prosecdef OR
p1.proleakproof != p2.proleakproof OR
p1.proisstrict != p2.proisstrict OR
p1.proretset != p2.proretset OR
p1.provolatile != p2.provolatile OR
p1.pronargs != p2.pronargs)
ORDER BY p1.oid, p2.oid; -- requires explicit ORDER BY now
FROM pg_proc AS p1, pg_proc AS p2
WHERE p1.oid < p2.oid AND
p1.prosrc = p2.prosrc AND
p1.prolang = 12 AND p2.prolang = 12 AND
(p1.prokind != 'a' OR p2.prokind != 'a') AND
(p1.prolang != p2.prolang OR
p1.prokind != p2.prokind OR
p1.prosecdef != p2.prosecdef OR
p1.proleakproof != p2.proleakproof OR
p1.proisstrict != p2.proisstrict OR
p1.proretset != p2.proretset OR
p1.provolatile != p2.provolatile OR
p1.pronargs != p2.pronargs)
ORDER BY p1.oid, p2.oid; -- requires explicit ORDER BY now
+ rangeTypeId = get_fn_expr_argtype(fcinfo->flinfo, 1);
+ if (!type_is_range(rangeTypeId))
+ {
+ ereport(ERROR, (errmsg("range_agg must be called with a range")));
+ }
+ if (!type_is_range(rangeTypeId))
+ {
+ ereport(ERROR, (errmsg("range_agg must be called with a range")));
+ }
???
+ r1Str = "lastRange";
+ r2Str = "currentRange";
+ // TODO: Why is this segfaulting?:
+ // r1Str = DatumGetCString(DirectFunctionCall1(range_out, RangeTypePGetDatum(lastRange)));
+ // r2Str = DatumGetCString(DirectFunctionCall1(range_out, RangeTypePGetDatum(currentRange)));
+ ereport(ERROR, (errmsg("range_agg: gap detected between %s and %s", r1Str, r2Str)));
+ }
+ r2Str = "currentRange";
+ // TODO: Why is this segfaulting?:
+ // r1Str = DatumGetCString(DirectFunctionCall1(range_out, RangeTypePGetDatum(lastRange)));
+ // r2Str = DatumGetCString(DirectFunctionCall1(range_out, RangeTypePGetDatum(currentRange)));
+ ereport(ERROR, (errmsg("range_agg: gap detected between %s and %s", r1Str, r2Str)));
+ }
???
The patch doesn't respect Postgres formatting
+# Making 2- and 3-param range_agg polymorphic is difficult
+# because it would take an anyrange and return an anyrange[],
+# which doesn't exist.
+# As a workaround we define separate functions for each built-in range type.
+# This is what causes the mess in src/test/regress/expected/opr_sanity.out.
+{ oid => '8003', descr => 'aggregate transition function',
+# because it would take an anyrange and return an anyrange[],
+# which doesn't exist.
+# As a workaround we define separate functions for each built-in range type.
+# This is what causes the mess in src/test/regress/expected/opr_sanity.out.
+{ oid => '8003', descr => 'aggregate transition function',
This is strange. Does it means so range_agg will not work with custom range types?
I am not sure about implementation. I think so accumulating all ranges, sorting and processing on the end can be memory and CPU expensive.
Regards
Pavel
[1] No female committers yet ... is this 2019?
--
Álvaro Herrera https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
pgsql-hackers by date: