review: https://commitfest.postgresql.org/action/patch_view?id=1484
Hello I did review of this patch, that add three functions varwidth_bucket for types: anyelement, double and bigint * This patch respects PostgreSQL coding rules * it can applied without any issues * there are no new compile warnings * patch contains documentation and tests * all tests was passed * there was no any objection in related discussion. Functionality is clean and based on current functionality of width_bucket. 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. * 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. Regards Pavel