From: | Petr Jelinek <petr(at)2ndquadrant(dot)com> |
---|---|
To: | Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)postgresql(dot)org> |
Cc: | Robert Haas <robertmhaas(at)gmail(dot)com> |
Subject: | Re: review: Built-in binning functions |
Date: | 2014-06-22 11:02:53 |
Message-ID: | 53A6B7DD.7050503@2ndquadrant.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
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
From | Date | Subject | |
---|---|---|---|
Next Message | Simon Riggs | 2014-06-22 11:38:04 | Re: [RFC, POC] Don't require a NBuffer sized PrivateRefCount array of local buffer pins |
Previous Message | Dean Rasheed | 2014-06-22 10:38:32 | Re: API change advice: Passing plan invalidation info from the rewriter into the planner? |