2014-06-22 13:02 GMT+02:00 Petr Jelinek <p...@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 >
