Обсуждение: BUG #12885: The result of casting a double to an integer depends on the database version

Поиск
Список
Период
Сортировка

BUG #12885: The result of casting a double to an integer depends on the database version

От
rschaaf@commoninf.com
Дата:
The following bug has been logged on the website:

Bug reference:      12885
Logged by:          Rich Schaaf
Email address:      rschaaf@commoninf.com
PostgreSQL version: 9.4.1
Operating system:   Windows 7
Description:

The result returned by the following query appears to depend on the
PostgreSQL database version.

The query is:
select column1 AS double_value, cast(column1 AS INT) AS int_value
  from (VALUES (-2.5::double precision),
               (-1.5::double precision),
               (-0.5::double precision),
               (0.5::double precision),
               (1.5::double precision),
               (2.5::double precision)) t;

In PostgreSQL 9.3.5, the query returns:
double_value, int_value
  -2.5, -3
  -1.5, -2
  -0.5, -1
  0.5, 1
  1.5, 2
  2.5, 3

PostgreSQL 9.4.1, the query returns:
  -2.5, -2
  -1.5, -2
  -0.5, 0
  0.5, 0
  1.5, 2
  2.5, 2
rschaaf@commoninf.com writes:
> The result returned by the following query appears to depend on the
> PostgreSQL database version.

> The query is:
> select column1 AS double_value, cast(column1 AS INT) AS int_value
>   from (VALUES (-2.5::double precision),
>                (-1.5::double precision),
>                (-0.5::double precision),
>                (0.5::double precision),
>                (1.5::double precision),
>                (2.5::double precision)) t;

> In PostgreSQL 9.3.5, the query returns:
> double_value, int_value
>   -2.5, -3
>   -1.5, -2
>   -0.5, -1
>   0.5, 1
>   1.5, 2
>   2.5, 3

> PostgreSQL 9.4.1, the query returns:
>   -2.5, -2
>   -1.5, -2
>   -0.5, 0
>   0.5, 0
>   1.5, 2
>   2.5, 2

FWIW, I get the latter behavior (round to nearest even) in all release
branches, and I would say that one is correct.  Not real sure why your
9.3 installation is misbehaving.

            regards, tom lane

Re: BUG #12885: The result of casting a double to an integer depends on the database version

От
Michael Paquier
Дата:
On Mon, Mar 23, 2015 at 7:48 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> rschaaf@commoninf.com writes:
>> The result returned by the following query appears to depend on the
>> PostgreSQL database version.
>
>> The query is:
>> select column1 AS double_value, cast(column1 AS INT) AS int_value
>>   from (VALUES (-2.5::double precision),
>>                (-1.5::double precision),
>>                (-0.5::double precision),
>>                (0.5::double precision),
>>                (1.5::double precision),
>>                (2.5::double precision)) t;
>
>> In PostgreSQL 9.3.5, the query returns:
>> double_value, int_value
>>   -2.5, -3
>>   -1.5, -2
>>   -0.5, -1
>>   0.5, 1
>>   1.5, 2
>>   2.5, 3
>
>> PostgreSQL 9.4.1, the query returns:
>>   -2.5, -2
>>   -1.5, -2
>>   -0.5, 0
>>   0.5, 0
>>   1.5, 2
>>   2.5, 2
>
> FWIW, I get the latter behavior (round to nearest even) in all release
> branches, and I would say that one is correct.  Not real sure why your
> 9.3 installation is misbehaving.

On a Windows 7 box with code compiled with MSVC 2010 I am seeing the
same behavior as Rich. This looks like a bug in ~9.3 that meritates
some attention assuming that the latter behavior is legit.
--
Michael

Re: BUG #12885: The result of casting a double to an integer depends on the database version

От
Michael Paquier
Дата:
On Tue, Mar 24, 2015 at 11:27 AM, Michael Paquier wrote:
> On a Windows 7 box with code compiled with MSVC 2010 I am seeing the
> same behavior as Rich. This looks like a bug in ~9.3 that meritates
> some attention assuming that the latter behavior is legit.

And MinGW outputs the latter, similarly to other platforms... I'll investigate.
--
Michael
Michael Paquier <michael.paquier@gmail.com> writes:
> On Tue, Mar 24, 2015 at 11:27 AM, Michael Paquier wrote:
>> On a Windows 7 box with code compiled with MSVC 2010 I am seeing the
>> same behavior as Rich. This looks like a bug in ~9.3 that meritates
>> some attention assuming that the latter behavior is legit.

> And MinGW outputs the latter, similarly to other platforms... I'll investigate.

Look for something about setting the IEEE float rounding mode.  "Round to
nearest even" is standard in most places, but it would not astonish me
to hear that Microsoft got that wrong.

If it is wrong I don't know that we'd want to back-patch a behavioral
change, but I'd definitely vote for conforming to the norm in 9.5
and later.

            regards, tom lane

Re: BUG #12885: The result of casting a double to an integer depends on the database version

От
Michael Paquier
Дата:
On Mon, Mar 23, 2015 at 8:12 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> Michael Paquier <michael.paquier@gmail.com> writes:
>> On Tue, Mar 24, 2015 at 11:27 AM, Michael Paquier wrote:
>>> On a Windows 7 box with code compiled with MSVC 2010 I am seeing the
>>> same behavior as Rich. This looks like a bug in ~9.3 that meritates
>>> some attention assuming that the latter behavior is legit.
>
>> And MinGW outputs the latter, similarly to other platforms... I'll investigate.
>
> Look for something about setting the IEEE float rounding mode.  "Round to
> nearest even" is standard in most places, but it would not astonish me
> to hear that Microsoft got that wrong.
>
> If it is wrong I don't know that we'd want to back-patch a behavioral
> change, but I'd definitely vote for conforming to the norm in 9.5
> and later.

I have done more testing (done only 9.3 with MinGW and MSVC before)
and the failure can be reproduced on master and REL9_4_STABLE as well
because visibly with MSVC 2010 the version of rint used is the one of
src/port, which is defined like that:
return (x >= 0.0) ? floor(x + 0.5) : ceil(x - 0.5);
For example by passing 2.5, we would get 3.0, and not 2.0 (nearest
even number). So isn't the problem here?
--
Michael
Michael Paquier <michael.paquier@gmail.com> writes:
> On Mon, Mar 23, 2015 at 8:12 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> Look for something about setting the IEEE float rounding mode.  "Round to
>> nearest even" is standard in most places, but it would not astonish me
>> to hear that Microsoft got that wrong.

> I have done more testing (done only 9.3 with MinGW and MSVC before)
> and the failure can be reproduced on master and REL9_4_STABLE as well
> because visibly with MSVC 2010 the version of rint used is the one of
> src/port, which is defined like that:
> return (x >= 0.0) ? floor(x + 0.5) : ceil(x - 0.5);
> For example by passing 2.5, we would get 3.0, and not 2.0 (nearest
> even number). So isn't the problem here?

Hmm ... why are we using src/port's version?  rint() has been required
by POSIX for decades, surely MSVC has got it?

(IOW, I'd rather fix this by removing src/port's version than by
trying to upgrade it to full IEEE spec.)

            regards, tom lane

Re: BUG #12885: The result of casting a double to an integer depends on the database version

От
Michael Paquier
Дата:
On Tue, Mar 24, 2015 at 10:37 PM, Tom Lane wrote:
> Michael Paquier <michael.paquier@gmail.com> writes:
>> For example by passing 2.5, we would get 3.0, and not 2.0 (nearest
>> even number). So isn't the problem here?
>
> Hmm ... why are we using src/port's version?  rint() has been required
> by POSIX for decades, surely MSVC has got it?
>
> (IOW, I'd rather fix this by removing src/port's version than by
> trying to upgrade it to full IEEE spec.)

rint() has been added in MSVC 2013, per se for example this thing
introduced by cec8394:
src/tools/msvc/Mkvcbuild.pm:    push(@pgportfiles, 'rint.c') if
($vsVersion < '12.00');

So I would imagine that in a majority of cases the version of rint()
used by an MSVC build on Windows is src/port/rint.c, not to mention
that on platforms where rint() is not available (the world is full of
surprises), the behavior would be incorrect. It is possible to blame
Redmond's folks about missing rint() from MSVC for more than 10 years,
but the blame could be put on PG-side as well.

Hence I'd rather think that patching src/port is the way to go, with
for example something like the patch attached. That's a behavioral
change, so maybe a backpatch is not welcome. See for example Rich's
email meaning that 9.4 behavior is broken as I understand it, but
that's actually the other way around, and I suspect that the version
of 9.4 Rich has used was built with at least MSVC 2013, while the
version of 9.3 he used was built with src/port/rint.c.
--
Michael

Вложения

Re: BUG #12885: The result of casting a double to an integer depends on the database version

От
Michael Paquier
Дата:
On Tue, Mar 24, 2015 at 7:11 PM, Pedro Gimeno
<pgsql-004@personal.formauri.es> wrote:
> Michael Paquier wrote, On 2015-03-25 01:19:
>> Hence I'd rather think that patching src/port is the way to go, with
>> for example something like the patch attached.
>
> There are some corner cases that that patch does not handle properly,
> most notably doubles that would overflow an integer. I suggest the
> attached implementation (it's not as a patch, though). I've included a
> test suite. The function can deal with non-IEEE floats too, but some of
> the tests assume IEEE doubles.

copysign is not that portable, at least it is not in the MSVC world.
So as a patch you would get something like the attached with what you
wrote.
--
Michael

Вложения

Re: BUG #12885: The result of casting a double to an integer depends on the database version

От
Pedro Gimeno
Дата:
Michael Paquier wrote, On 2015-03-25 01:19:
> Hence I'd rather think that patching src/port is the way to go, with
> for example something like the patch attached.

There are some corner cases that that patch does not handle properly,
most notably doubles that would overflow an integer. I suggest the
attached implementation (it's not as a patch, though). I've included a
test suite. The function can deal with non-IEEE floats too, but some of
the tests assume IEEE doubles.


Вложения

Re: BUG #12885: The result of casting a double to an integer depends on the database version

От
Michael Paquier
Дата:
On Wed, Mar 25, 2015 at 8:18 PM, Pedro Gimeno
<pgsql-004@personal.formauri.es> wrote:
> Michael Paquier wrote, On 2015-03-25 04:26:
>> copysign is not that portable, at least it is not in the MSVC world.
>> So as a patch you would get something like the attached with what you
>> wrote.
>
> The only point of using copysign there was to deal with a negative zero
> corner case. The attached removes copysign from the test program too,
> relying instead on sprintf outputting the sign of minus zero when available.

Cool, thanks. Applied to the code tree of Postgres, it gives the patch attached.
--
Michael

Вложения
Michael Paquier <michael.paquier@gmail.com> writes:
> On Wed, Mar 25, 2015 at 8:18 PM, Pedro Gimeno
> <pgsql-004@personal.formauri.es> wrote:
>> The only point of using copysign there was to deal with a negative zero
>> corner case. The attached removes copysign from the test program too,
>> relying instead on sprintf outputting the sign of minus zero when available.

> Cool, thanks. Applied to the code tree of Postgres, it gives the patch attached.

This is pretty desperately in need of some comments, but I'll see what
I can do with it.

I assume the consensus is that we should not back-patch this?  It seems
like the kind of behavioral change that people might get annoyed about.

            regards, tom lane

Re: BUG #12885: The result of casting a double to an integer depends on the database version

От
Pedro Gimeno
Дата:
Michael Paquier wrote, On 2015-03-25 04:26:
> copysign is not that portable, at least it is not in the MSVC world.
> So as a patch you would get something like the attached with what you
> wrote.

Here's a version without copysign.


Вложения

Re: BUG #12885: The result of casting a double to an integer depends on the database version

От
Pedro Gimeno
Дата:
Michael Paquier wrote, On 2015-03-25 04:26:
> copysign is not that portable, at least it is not in the MSVC world.
> So as a patch you would get something like the attached with what you
> wrote.

The only point of using copysign there was to deal with a negative zero
corner case. The attached removes copysign from the test program too,
relying instead on sprintf outputting the sign of minus zero when available.


Вложения
Pedro Gimeno <pgsql-004@personal.formauri.es> writes:
> The previous version left out a corner case. Attached is a function and
> test suite to deal with it.

Hmm ... I'm thinking we probably should explicitly check for inf and NaN,
no?

    if (isnan(x) || isinf(x))
       return x;

It's possible the given coding would return this result anyway by
accident, but that seems rather fragile.

            regards, tom lane
Pedro Gimeno <pgsql-004@personal.formauri.es> writes:
> Tom Lane wrote, On 2015-03-25 18:57:
>> Hmm ... I'm thinking we probably should explicitly check for inf and NaN,
>> no?

> I agree about NaN; it worked but mostly by accident. The big number
> detection catches infinity unambiguously. Added to a comment in the
> attached (plus corresponding tests).

I fooled around with this some more for clarity and committed it.
Thanks for your work!

BTW, if memory serves we also have a pretty lazy rounding implementation
for the numeric datatype.  I wonder if now would be a good time to upgrade
that to be round-to-nearest-even as well.

            regards, tom lane

Re: Re: BUG #12885: The result of casting a double to an integer depends on the database version

От
Michael Paquier
Дата:
On Thu, Mar 26, 2015 at 4:57 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> Pedro Gimeno <pgsql-004@personal.formauri.es> writes:
>> Tom Lane wrote, On 2015-03-25 18:57:
>>> Hmm ... I'm thinking we probably should explicitly check for inf and NaN,
>>> no?
>
>> I agree about NaN; it worked but mostly by accident. The big number
>> detection catches infinity unambiguously. Added to a comment in the
>> attached (plus corresponding tests).
>
> I fooled around with this some more for clarity and committed it.
> Thanks for your work!

Thanks Tom for wrapping up stuff, and Pedro for the patch.
--
Michael

Re: Re: BUG #12885: The result of casting a double to an integer depends on the database version

От
Andrew Gierth
Дата:
>>>>> "Tom" == Tom Lane <tgl@sss.pgh.pa.us> writes:

 Tom> BTW, if memory serves we also have a pretty lazy rounding
 Tom> implementation for the numeric datatype.  I wonder if now would be
 Tom> a good time to upgrade that to be round-to-nearest-even as well.

A data point: there have been occasional complaints on IRC about the
fact that numeric rounding is not round-to-even, but obviously it's
harder to tell if anyone is relying on the current behavior.

--
Andrew (irc:RhodiumToad)

Re: Re: BUG #12885: The result of casting a double to an integer depends on the database version

От
Michael Paquier
Дата:
On Thu, Mar 26, 2015 at 9:19 AM, Andrew Gierth
<andrew@tao11.riddles.org.uk> wrote:
>>>>>> "Tom" == Tom Lane <tgl@sss.pgh.pa.us> writes:
>
>  Tom> BTW, if memory serves we also have a pretty lazy rounding
>  Tom> implementation for the numeric datatype.  I wonder if now would be
>  Tom> a good time to upgrade that to be round-to-nearest-even as well.
>
> A data point: there have been occasional complaints on IRC about the
> fact that numeric rounding is not round-to-even, but obviously it's
> harder to tell if anyone is relying on the current behavior.

Indeed...
=# select column1 AS double_value, cast(column1 AS INT) AS int_value
  FROM (VALUES (-2.5::numeric),
         (-1.5::numeric),
         (-0.5::numeric),
         (0.5::numeric),
         (1.5::numeric),
         (2.5::numeric)) t;
 double_value | int_value
--------------+-----------
         -2.5 |        -3
         -1.5 |        -2
         -0.5 |        -1
          0.5 |         1
          1.5 |         2
          2.5 |         3
(6 rows)

Tom, if there is a patch showing up soon, would you integrate it? I
imagine that it would be good to have the same behavior for a maximum
of datatypes in 9.5 now that src/port/rint.c is more compliant.
Regards,
--
Michael
Michael Paquier <michael.paquier@gmail.com> writes:
> On Thu, Mar 26, 2015 at 9:19 AM, Andrew Gierth
> <andrew@tao11.riddles.org.uk> wrote:
> "Tom" == Tom Lane <tgl@sss.pgh.pa.us> writes:
>>> Tom> BTW, if memory serves we also have a pretty lazy rounding
>>> Tom> implementation for the numeric datatype.  I wonder if now would be
>>> Tom> a good time to upgrade that to be round-to-nearest-even as well.

>> A data point: there have been occasional complaints on IRC about the
>> fact that numeric rounding is not round-to-even, but obviously it's
>> harder to tell if anyone is relying on the current behavior.

> Tom, if there is a patch showing up soon, would you integrate it? I
> imagine that it would be good to have the same behavior for a maximum
> of datatypes in 9.5 now that src/port/rint.c is more compliant.

Yes, that's what I was thinking --- if we're changing this in 9.5 for
float (on Windows and whatever other platforms use rint.c) then it would
be sensible to fix numeric at the same time.  I will commit a patch if
it shows up, but do not have time to write one myself.

            regards, tom lane

Re: Re: BUG #12885: The result of casting a double to an integer depends on the database version

От
Andrew Gierth
Дата:
>>>>> "Michael" == Michael Paquier <michael.paquier@gmail.com> writes:

 Michael> Indeed...
 Michael> =# select column1 AS double_value, cast(column1 AS INT) AS
 Michael> int_value

The complication for numeric is that there's also the case of round(x,n)
and casting to numeric(m,n) to consider.

--
Andrew (irc:RhodiumToad)

Re: Re: BUG #12885: The result of casting a double to an integer depends on the database version

От
Michael Paquier
Дата:
On Thu, Mar 26, 2015 at 11:12 AM, Andrew Gierth wrote:
>>>>>> "Michael" == Michael Paquier writes:
>
>  Michael> Indeed...
>  Michael> =# select column1 AS double_value, cast(column1 AS INT) AS
>  Michael> int_value
>
> The complication for numeric is that there's also the case of round(x,n)
> and casting to numeric(m,n) to consider.

OK, thanks for the reminder... I forgot this case.
--
Michael

Re: Re: BUG #12885: The result of casting a double to an integer depends on the database version

От
Pedro Gimeno
Дата:
Tom Lane wrote, On 2015-03-25 15:37:
> This is pretty desperately in need of some comments, but I'll see what
> I can do with it.

Sorry about that. I hope the attached helps.


Вложения

Re: Re: BUG #12885: The result of casting a double to an integer depends on the database version

От
Pedro Gimeno
Дата:
Pedro Gimeno wrote, On 2015-03-25 17:22:
> Sorry about that. I hope the attached helps.

The previous version left out a corner case. Attached is a function and
test suite to deal with it.


Вложения

Re: Re: BUG #12885: The result of casting a double to an integer depends on the database version

От
Pedro Gimeno
Дата:
Tom Lane wrote, On 2015-03-25 18:57:
> Hmm ... I'm thinking we probably should explicitly check for inf and NaN,
> no?
>
>     if (isnan(x) || isinf(x))
>        return x;
>
> It's possible the given coding would return this result anyway by
> accident, but that seems rather fragile.

I agree about NaN; it worked but mostly by accident. The big number
detection catches infinity unambiguously. Added to a comment in the
attached (plus corresponding tests).


Вложения

Re: Re: BUG #12885: The result of casting a double to an integer depends on the database version

От
Michael Paquier
Дата:
On Thu, Mar 26, 2015 at 11:15 AM, Michael Paquier
<michael.paquier@gmail.com> wrote:
> On Thu, Mar 26, 2015 at 11:12 AM, Andrew Gierth wrote:
>>>>>>> "Michael" == Michael Paquier writes:
>>
>>  Michael> Indeed...
>>  Michael> =# select column1 AS double_value, cast(column1 AS INT) AS
>>  Michael> int_value
>>
>> The complication for numeric is that there's also the case of round(x,n)
>> and casting to numeric(m,n) to consider.
>
> OK, thanks for the reminder... I forgot this case.

Attached is a patch that adds nearest-to-even rounding for numeric
when dscale = 0. This gives the following results with the previous
query:
=# select column1 AS double_value, cast(column1 AS INT) AS int_value
                                                          FROM (VALUES
(-2.5::numeric),

(-1.5::numeric),

                       (-0.5::numeric),

                                              (0.5::numeric),


(1.5::numeric),

                        2.5::numeric)) t;
 double_value | int_value
--------------+-----------
         -2.5 |        -2
         -1.5 |        -2
         -0.5 |         0
          0.5 |         0
          1.5 |         2
          2.5 |         2
(6 rows)

For the case where dscale != 0 case, what would we actually want? I
have been staring at the code around round_var for some time but I
could not really get if we'd really want to change something there...
For example round(2.5::numeric, 1) should be 2.4 and not 2.5?
Regards.
--
Michael

Вложения

Re: Re: BUG #12885: The result of casting a double to an integer depends on the database version

От
Dean Rasheed
Дата:
On 27 March 2015 at 12:32, Michael Paquier <michael.paquier@gmail.com> wrote:
> On Thu, Mar 26, 2015 at 11:15 AM, Michael Paquier
> <michael.paquier@gmail.com> wrote:
>> On Thu, Mar 26, 2015 at 11:12 AM, Andrew Gierth wrote:
>>>>>>>> "Michael" == Michael Paquier writes:
>>>
>>>  Michael> Indeed...
>>>  Michael> =# select column1 AS double_value, cast(column1 AS INT) AS
>>>  Michael> int_value
>>>
>>> The complication for numeric is that there's also the case of round(x,n)
>>> and casting to numeric(m,n) to consider.
>>
>> OK, thanks for the reminder... I forgot this case.
>
> Attached is a patch that adds nearest-to-even rounding for numeric
> when dscale = 0.

I'm not convinced that it is a good idea to change the default
rounding mode for numeric.

For one thing, numeric is commonly used for monetary data, and
changing the default rounding mode might well break existing
applications where rounding is important, and which rely on the
current behaviour. Granted the current rounding mode doesn't appear to
be documented anywhere (which it probably should be), but still, it
seems like a risky thing to change.

I'd also argue that the current "round half away from zero" mode is
the most widely known rounding mode, since it is commonly taught at
school, so it seems like the more sensible default.

ISTM that there would have to be a good reason to change the default,
and compatibility with IEEE floats doesn't seem very convincing to me.
I would never assume numerics are meant to have anything in common
with IEEE floats.

Adding a version of round() with support for a choice of various
rounding modes might be a better way to go.

Also, if the default is being changed, I think that merits wider
discussion (on a thread with a more appropriate title) to see if it is
likely to be an issue for anyone.

Regards,
Dean

Re: Re: BUG #12885: The result of casting a double to an integer depends on the database version

От
Michael Paquier
Дата:
On Sat, Mar 28, 2015 at 2:53 AM, Pedro Gimeno
<pgsql-004@personal.formauri.es> wrote:
> Michael Paquier wrote, On 2015-03-27 13:32:
>
>> Attached is a patch that adds nearest-to-even rounding for numeric
>> when dscale = 0. This gives the following results with the previous
>> query:
>
> That produces some quite surprising results:
>
> =# select '2.5'::numeric(9,0)::numeric(9,1),
>          '2.25'::numeric(9,1)::numeric(9,2),
>         '2.225'::numeric(9,2)::numeric(9,3),
>        '2.2225'::numeric(9,3)::numeric(9,4),
>       '2.22225'::numeric(9,4)::numeric(9,5);
>  numeric | numeric | numeric | numeric | numeric
> ---------+---------+---------+---------+---------
>      2.0 |    2.30 |   2.230 |  2.2230 | 2.22220
> (1 row)
>
> That rounding is inconsistent: since DEC_DIGITS is 4 by default, numbers
> with a number of digits which is a multiple of 4 + 1 will be rounded to
> nearest or even, and numbers with a different number of digits will be
> rounded up on a tie. And I have not tested, but apparently when
> DEC_DIGITS == 1 (array elements are single digits rather than packing
> several) it always rounds down on a tie.

Yeah, I forgot a check on dscale = 0 when deciding to carry to the
upper digit or not.
--
Michael

Re: Re: BUG #12885: The result of casting a double to an integer depends on the database version

От
Michael Paquier
Дата:
On Sat, Mar 28, 2015 at 2:16 AM, Dean Rasheed wrote:
> On 27 March 2015 at 12:32, Michael Paquier wrote:
>> Attached is a patch that adds nearest-to-even rounding for numeric
>> when dscale = 0.
>
> I'm not convinced that it is a good idea to change the default
> rounding mode for numeric.
>
> For one thing, numeric is commonly used for monetary data, and
> changing the default rounding mode might well break existing
> applications where rounding is important, and which rely on the
> current behaviour. Granted the current rounding mode doesn't appear to
> be documented anywhere (which it probably should be), but still, it
> seems like a risky thing to change.

It is possible to argue the other way around here. For people
migrating to Postgres, perhaps the behavior that we have is surprising
as well. Perhaps other RDBMS behave using rounding-to-even.

> I'd also argue that the current "round half away from zero" mode is
> the most widely known rounding mode, since it is commonly taught at
> school, so it seems like the more sensible default.
>
> ISTM that there would have to be a good reason to change the default,
> and compatibility with IEEE floats doesn't seem very convincing to me.
> I would never assume numerics are meant to have anything in common
> with IEEE floats.
>
> Adding a version of round() with support for a choice of various
> rounding modes might be a better way to go.

Each argument has its strengths. Consistency with float and rint() is
appealing as well...

> Also, if the default is being changed, I think that merits wider
> discussion (on a thread with a more appropriate title) to see if it is
> likely to be an issue for anyone.

Yeah, that makes sense. I'll raise one on -hackers.
--
Michael

Re: Re: BUG #12885: The result of casting a double to an integer depends on the database version

От
Andrew Gierth
Дата:
>>>>> "Dean" == Dean Rasheed <dean.a.rasheed@gmail.com> writes:

 Dean> I'm not convinced that it is a good idea to change the default
 Dean> rounding mode for numeric.

 Dean> For one thing, numeric is commonly used for monetary data, and
 Dean> changing the default rounding mode might well break existing
 Dean> applications where rounding is important, and which rely on the
 Dean> current behaviour.

On the other hand, we've had complaints (on IRC) from people doing
calculations with monetary data who complain that we don't do
round-to-even (which is widely known as "Banker's Rounding") as their
requirements demand.

To quote the last person who brought it up there:

 >> We have strict accounting practice to follow -- and our CFO has
 >> instructed to use even-rounding (aka bankers rounding)... and to not
 >> use our typical rounding behavior we learned in 1st grade. :)

 Dean> Adding a version of round() with support for a choice of various
 Dean> rounding modes might be a better way to go.

There's still the issue of casting to ::numeric(m,n) to consider, and
whatever internal rounding is being done in division and numeric
functions.

 Dean> Also, if the default is being changed, I think that merits wider
 Dean> discussion (on a thread with a more appropriate title) to see if
 Dean> it is likely to be an issue for anyone.

Absolutely.

--
Andrew (irc:RhodiumToad)

Re: Re: BUG #12885: The result of casting a double to an integer depends on the database version

От
Pedro Gimeno
Дата:
Michael Paquier wrote, On 2015-03-27 13:32:

> Attached is a patch that adds nearest-to-even rounding for numeric
> when dscale = 0. This gives the following results with the previous
> query:

That produces some quite surprising results:

=# select '2.5'::numeric(9,0)::numeric(9,1),
         '2.25'::numeric(9,1)::numeric(9,2),
        '2.225'::numeric(9,2)::numeric(9,3),
       '2.2225'::numeric(9,3)::numeric(9,4),
      '2.22225'::numeric(9,4)::numeric(9,5);
 numeric | numeric | numeric | numeric | numeric
---------+---------+---------+---------+---------
     2.0 |    2.30 |   2.230 |  2.2230 | 2.22220
(1 row)

That rounding is inconsistent: since DEC_DIGITS is 4 by default, numbers
with a number of digits which is a multiple of 4 + 1 will be rounded to
nearest or even, and numbers with a different number of digits will be
rounded up on a tie. And I have not tested, but apparently when
DEC_DIGITS == 1 (array elements are single digits rather than packing
several) it always rounds down on a tie.