Re: range_agg

Поиск
Список
Период
Сортировка
От Pavel Stehule
Тема Re: range_agg
Дата
Msg-id CAFj8pRBZJ6aiLXOf7SDT=R4pscN--44YmGAaeP_qSr9eaS1MCA@mail.gmail.com
обсуждение исходный текст
Ответ на Re: range_agg  (Alvaro Herrera <alvherre@2ndquadrant.com>)
Ответы Re: range_agg  (Paul A Jungwirth <pj@illuminatedcomputing.com>)
Список 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

+   rangeTypeId = get_fn_expr_argtype(fcinfo->flinfo, 1);
+   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)));
+               }

???

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',

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 по дате отправления:

Предыдущее
От: Dmitry Dolgov
Дата:
Сообщение: Re: Index Skip Scan
Следующее
От: Peter Eisentraut
Дата:
Сообщение: Re: how to run encoding-dependent tests by default