From: | David Fetter <david(at)fetter(dot)org> |
---|---|
To: | Andrew Gierth <andrew(at)tao11(dot)riddles(dot)org(dot)uk> |
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-23 21:35:07 |
Message-ID: | 20190923213506.GN31596@fetter.org |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
On Mon, Sep 23, 2019 at 01:16:36PM +0100, Andrew Gierth wrote:
> >>>>> "David" == David Fetter <david(at)fetter(dot)org> writes:
>
> David> + return pg_ltostr_zeropad(str, (uint32)0 - (uint32)value, minwidth - 1);
>
> No, this is just reintroducing the undefined behavior again. Once the
> value has been converted to unsigned you can't cast it back to signed or
> pass it to a function expecting a signed value, since it will overflow
> in the INT_MIN case. (and in this example would probably output '-'
> signs until it ran off the end of memory).
>
> Here's how I would do it:
>
> char *
> pg_ltostr_zeropad(char *str, int32 value, int32 minwidth)
> {
> int32 len;
> uint32 uvalue = value;
>
> Assert(minwidth > 0);
>
> if (value >= 0)
> {
> if (value < 100 && minwidth == 2) /* Short cut for common case */
> {
> memcpy(str, DIGIT_TABLE + value*2, 2);
> return str + 2;
> }
> }
> else
> {
> *str++ = '-';
> minwidth -= 1;
> uvalue = (uint32)0 - uvalue;
> }
>
> len = pg_ultoa_n(uvalue, str);
> if (len >= minwidth)
> return str + len;
>
> memmove(str + minwidth - len, str, len);
> memset(str, '0', minwidth - len);
> return str + minwidth;
> }
Done pretty much that way.
> David> pg_ltostr(char *str, int32 value)
> David> + int32 len = pg_ultoa_n(value, str);
> David> + return str + len;
>
> This seems to have lost its handling of negative numbers entirely
Given the comment that precedes it and all the use cases in the code,
I changed the signature to take an unsigned integer instead. It's
pretty clear that the intent was to add digits and only digits to the
passed-in string.
> (which doesn't say much for the regression test coverage)
I didn't see any obvious way to test functions not surfaced to SQL.
Should we have one?
Best,
David.
--
David Fetter <david(at)fetter(dot)org> http://fetter.org/
Phone: +1 415 235 3778
Remember to vote!
Consider donating to Postgres: http://www.postgresql.org/about/donate
Attachment | Content-Type | Size |
---|---|---|
v12-0001-Make-int4-and-int8-operations-more-efficent.patch | text/x-diff | 13.6 KB |
From | Date | Subject | |
---|---|---|---|
Next Message | Jungle Boogie | 2019-09-23 21:43:37 | Re: scorpionfly needs more semaphores |
Previous Message | Benjie Gillam | 2019-09-23 21:34:07 | [PATCH] Sort policies and triggers by table name in pg_dump. |