New patch attached and rebased on HEAD
(8c75ad436f75fc629b61f601ba884c8f9313c9af).
Am 03.11.2015 um 04:06 schrieb Robert Haas:
> On Sat, Oct 31, 2015 at 2:25 PM, Julien Rouhaud
> <julien.rouhaud@dalibo.com> wrote:
>> I just reviewed your patch, everything looks fine for me. Maybe some
>> minor cosmetic changes could be made to avoid declaring too many vars,
>> but I think a committer would have a better idea on this, so I mark
>> this patch as ready for committer.
>
> I don't think we should define Sign(x) as a macro in c.h. c.h is
> really only supposed to contain stuff that's pretty generic and
> universal, and the fact that we haven't needed it up until now
> suggests that Sign(x) isn't. I'd suggest instead defining something
> like:
>
> #define half_rounded(x) (((x) + (x) < 0 ? 0 : 1) / 2)
I removed the Sign macro and introduced half_rounded. It's placed it
in dbsize.c. Please let me know if that's the wrong place.
>
> Maybe rename numeric_divide_by_two to numeric_half_rounded.
> Generally, let's try to make the numeric and int64 code paths look as
> similar as possible.
I renamed numeric_divide_by_two.
>
> Recomputing numeric_absolute(x) multiple times should be avoided.
> Compute it once and save the answer.
Because "size" is shifted multiple times within pg_size_pretty and
pg_size_pretty_numeric the absolute values needs to be recomputed.
Please let me know if I oversee something.
I changed the status back to "needs review".
Regards,
- Adrian
