Re: Efficient output for integer types

From: Andrew Gierth <andrew(at)tao11(dot)riddles(dot)org(dot)uk>
To: David Fetter <david(at)fetter(dot)org>
Cc: Kyotaro Horiguchi <horikyota(dot)ntt(at)gmail(dot)com>, pgsql-hackers(at)postgresql(dot)org
Subject: Re: Efficient output for integer types
Date: 2019-09-21 06:29:25
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

>>>>> "David" == David Fetter <david(at)fetter(dot)org> writes:

David> +static inline uint32
David> +decimalLength64(const uint64_t v)

Should be uint64, not uint64_t.

Also return an int, not a uint32.

For int vs. int32, my own inclination is to use "int" where the value is
just a (smallish) number, especially one that will be used as an index
or loop count, and use "int32" when it actually matters that it's 32
bits rather than some other size. Other opinions may differ.

David> +{
David> + uint32 t;
David> + static uint64_t PowersOfTen[] = {

uint64 not uint64_t here too.

David> +int32
David> +pg_ltoa_n(uint32 value, char *a)

If this is going to handle only unsigned values, it should probably be
named pg_ultoa_n.

David> + uint32 i = 0, adjust = 0;

"adjust" is not assigned anywhere else. Presumably that's from previous
handling of negative numbers?

David> + memcpy(a, "0", 1);

*a = '0'; would suffice.

David> + i += adjust;


David> + uint32_t uvalue = (uint32_t)value;

uint32 not uint32_t.

David> + int32 len;

See above re. int vs. int32.

David> + uvalue = (uint32_t)0 - (uint32_t)value;

Should be uint32 not uint32_t again.

For anyone wondering, I suggested this to David in place of the ugly
special casing of INT32_MIN. This method avoids the UB of doing (-value)
where value==INT32_MIN, and is nevertheless required to produce the
correct result:

1. If value < 0, then ((uint32)value) is (value + UINT32_MAX + 1)
2. (uint32)0 - (uint32)value
becomes (UINT32_MAX+1)-(value+UINT32_MAX+1)
which is (-value) as required

David> +int32
David> +pg_lltoa_n(uint64_t value, char *a)

Again, if this is doing unsigned, then it should be named pg_ulltoa_n

David> + if (value == PG_INT32_MIN)

This being inconsistent with the others is not nice.

Andrew (irc:RhodiumToad)

In response to


Browse pgsql-hackers by date

  From Date Subject
Next Message Fabien COELHO 2019-09-21 07:48:31 Re: pgbench - allow to create partitioned tables
Previous Message Dilip Kumar 2019-09-21 06:28:17 Re: A problem about partitionwise join