Thread: review: Built-in binning functions

Hi,

On 21/06/14 20:41, Pavel Stehule wrote:
> review: https://commitfest.postgresql.org/action/patch_view?id=1484
>

Thanks for review.

>
> My comments:
>
> * I miss in documentation description of implementation - its is based
> on binary searching, and when second parameter is unsorted array, then
> it returns some nonsense without any warning.
>

Right I did mean to mention that thresholds array must be sorted, but 
forgot about it when submitting.

> * Description for anyelement is buggy twice times
>
> "varwidth_bucket(5.35::numeric, ARRAY[1, 3, 4, 6]::numeric)"
>
> probably should be "varwidth_bucket(5.35::numeric, ARRAY[1, 3, 4,
> 6]::numeric[])"
>
> BUT it is converted to double precision, function with polymorphic
> parameters is not used. So it not respects a widh_buckets model:
>
> postgres=# \dfS width_bucket
>                                                     List of functions
>     Schema   │     Name     │ Result data type │
> Argument data types                      │  Type
>
────────────┼──────────────┼──────────────────┼───────────────────────────────────────────────────────────────┼────────
>   pg_catalog │ width_bucket │ integer          │ double precision,
> double precision, double precision, integer │ normal
>   pg_catalog │ width_bucket │ integer          │ numeric, numeric,
> numeric, integer                            │ normal
> (2 rows)
>
> There should be a interface for numeric type too. I am sure so important
> part of code for polymorphic type can be shared.
>

I wonder if it would be acceptable to just create pg_proc entry and 
point it to generic implementation (that's what I originally had, then I 
changed pg_proc entry to polymorphic types...)

--  Petr Jelinek                  http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training &
Services

Hi,

On 2014-06-21 20:41:43 +0200, Pavel Stehule wrote:
> review: https://commitfest.postgresql.org/action/patch_view?id=1484

Can you please not start new threads for reviews of smaller features?
Doing so makes following the discussion much harder. I'm fine with
changing the subject if the reply headers are left intact...

Greetings,

Andres Freund

-- Andres Freund                       http://www.2ndQuadrant.com/PostgreSQL Development, 24x7 Support, Training &
Services

Here is v2,

with fixed documentation and numeric version of the implementation.

--
  Petr Jelinek                  http://www.2ndQuadrant.com/
  PostgreSQL Development, 24x7 Support, Training & Services