Re: review: Built-in binning functions - Mailing list pgsql-hackers

From Pavel Stehule
Subject Re: review: Built-in binning functions
Date
Msg-id CAFj8pRBNC+=HzB7ErHYq1dkhos09oBe-=K-uobWNi_9q5Z=bqw@mail.gmail.com
Whole thread Raw
In response to Re: review: Built-in binning functions  (Petr Jelinek <petr@2ndquadrant.com>)
Responses Re: review: Built-in binning functions  (Petr Jelinek <petr@2ndquadrant.com>)
List pgsql-hackers



2014-06-22 13:02 GMT+02:00 Petr Jelinek <petr@2ndquadrant.com>:
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...)

probably not. But very simple wrapper is acceptable.

Pavel
 

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

pgsql-hackers by date:

Previous
From: Stephen Frost
Date:
Subject: Re: How about a proper TEMPORARY TABLESPACE?
Next
From: Pavel Stehule
Date:
Subject: Re: [Fwd: Re: proposal: new long psql parameter --on-error-stop]