Re: [HACKERS] Fix number skipping in to_number

On Thu, Aug 17, 2017 at 9:48 PM, Oliver Ford <ojford@gmail.com> wrote:
>> The tests you added pass for me but the int8 test now fails with the
>> following (this is from regression.diff after running
>> 'PG_REGRESS_DIFF_OPTS="-dU10" make check').  It looks like some new
>> whitespace is appearing on the right in the output of to_char().  I
>> didn't try to understand why.
>>
>> @@ -453,34 +453,34 @@
>>  ------------------+------------------
>>   4567890123456789 | 4567890123456789
>>  (1 row)
>>
>>  -- TO_CHAR()
>>  --
>>  SELECT '' AS to_char_1, to_char(q1, '9G999G999G999G999G999'),
>> to_char(q2, '9,999,999,999,999,999')
>>         FROM INT8_TBL;
>>   to_char_1 |        to_char         |        to_char
>>  -----------+------------------------+------------------------
>> -           |                    123 |                    456
>> +           |                    123 |               456
>>             |                    123 |  4,567,890,123,456,789
>> -           |  4,567,890,123,456,789 |                    123
>> +           |  4,567,890,123,456,789 |               123
>>             |  4,567,890,123,456,789 |  4,567,890,123,456,789
>>             |  4,567,890,123,456,789 | -4,567,890,123,456,789
>>  (5 rows)
>>
>>  SELECT '' AS to_char_2, to_char(q1, '9G999G999G999G999G999D999G999'),
>> to_char(q2, '9,999,999,999,999,999.999,999')
>>         FROM INT8_TBL;
>>   to_char_2 |            to_char             |            to_char
>>  -----------+--------------------------------+--------------------------------
>> -           |                    123.000,000 |                    456.000,000
>> +           |                    123.000,000 |               456.000,000
>>             |                    123.000,000 |  4,567,890,123,456,789.000,000
>> -           |  4,567,890,123,456,789.000,000 |                    123.000,000
>> +           |  4,567,890,123,456,789.000,000 |               123.000,000
>>             |  4,567,890,123,456,789.000,000 |  4,567,890,123,456,789.000,000
>>             |  4,567,890,123,456,789.000,000 | -4,567,890,123,456,789.000,000
>>  (5 rows)
>>
>>  SELECT '' AS to_char_3, to_char( (q1 * -1), '9999999999999999PR'),
>> to_char( (q2 * -1), '9999999999999999.999PR')
>>         FROM INT8_TBL;
>>   to_char_3 |      to_char       |        to_char
>>  -----------+--------------------+------------------------
>>             |              <123> |              <456.000>
>>             |              <123> | <4567890123456789.000>
>>
>
> That's strange, I can't replicate that issue on my Windows build. I've
> tried with 'PG_REGRESS_DIFF_OPTS="-dU10" make check' and all the tests
> (including int8) pass fine. The spacing in the results is perfectly
> normal. I wonder if something else on your build is causing this? I've
> also tried several "make check" options for different locales
> mentioned in the docs and they pass fine.

Hmm.  Just in case my macOS laptop (CC=Apple's clang,
LANG=en_NZ.UTF-8) was unduly affected by cosmic rays I tried on a
couple of nearby servers running FreeBSD 11.1 (CC=clang, LANG=<unset>)
and CentOS 7.3 (CC=gcc, LANG=en_US.utf-8) and got the same result:
int8 has lost some whitespace in to_char output.

It looks a bit like it has lost a leading space for every ',' that was
present in the format string but which didn't finish up getting output
(because the number was too small).  It looks a bit like that doesn't
happen for 'G', so let's compare the NUM_COMMA and NUM_G cases.  Ahh..
I'm not sure, but I think you might have a close-brace in the wrong
place here:

@@ -4879,6 +4883,10 @@ NUM_processor(FormatNode *node, NUMDesc *Num,
char *inout,                                                               continue;
          }                                       }    <--- this guy here might
 
need to move after "noadd = true"?
+                                               if
(strncmp(Np->inout_p, Np->L_thousands_sep, separator_len) == 0)
+                                                       Np->inout_p +=
separator_len - 1;
+                                               else
+                                                       noadd = true;                                       break;
                               case NUM_G:

With that change the test passes for me.  But why do we get different results?

-- 
Thomas Munro
http://www.enterprisedb.com