On Sat, Jul 25, 2009 at 2:08 AM, Brendan Jurd<direvus@gmail.com> wrote:
> 2009/7/24 Euler Taveira de Oliveira <euler@timbira.com>:
>> Here is my review. The patch applied without problems. The docs and regression
>> tests are included. Both of them worked as expected. Also, you included a fix
>> in RN format, do it in another patch.
>>
>
> Well, I updated an error message for RN to keep it consistent with the
> change I made to the nearby EEEE error message.
>
> Neither RN or EEEE is supported for input, and the error messages were
> vague on this point (they just said "not supported").
>
> I understand that separate improvements should be submitted as
> separate patches, but this is really part of the one improvement.
> Implementing EEEE required improving the error messages, and
> consistency required that we improve the RN error message also.
>
>> The behavior is not the same as Oracle. Oracle accepts an invalid scientific
>> notation '999.9EEEE'. Will we support it too? I think so.
>>
>> euler=# SELECT to_char(1234.56789, '999.9EEEE');
>> ERRO: invalid format for scientific notation
>> DETALHE: "EEEE" requires exactly one digit before the decimal point.
>> DICA: For example, "9.999EEEE" is a valid format.
>>
>> TO_CHAR(1234.56789,'999.9EEEE')
>> -------------------------------
>> 1.2E+03
>
> *shakes fist at Oracle* yes, I suppose we had better follow suit.
>
>> The '9.999eeee' format error message is misleading.
>>
>> euler=# select to_char(123, '9.999eeee');
>> ERRO: cannot use "EEEE" twice
>
> Ah, thanks for picking up on this. This was a bug in the original
> patch. Looks like we forgot to update the formatting keyword for
> lowercase "e".
>
>>
>> You could include an example in manual too. You could add the two failing
>> cases above in regression tests too.
>>
>
> I had already added an example to the manual.
>
> Please find attached version 4 of the patch, and incremental diff from
> version 3. It fixes the "eeee" bug ("eeee" is now accepted as a valid
> form of "EEEE"), and lifts the restriction on only having one digit
> before the decimal point.
Euler,
Are you going to review this again? We need to know if it is Ready
for Committer.
...Robert