[SERVER-6166] consider expanding to wider data type on math overflow Created: 21/Jun/12 Updated: 28/Oct/15 Resolved: 27/Jul/12 |
|
| Status: | Closed |
| Project: | Core Server |
| Component/s: | Aggregation Framework |
| Affects Version/s: | None |
| Fix Version/s: | 2.2.0-rc1 |
| Type: | Question | Priority: | Major - P3 |
| Reporter: | Aaron Staple | Assignee: | Matt Dannenberg |
| Resolution: | Done | Votes: | 0 |
| Labels: | None | ||
| Remaining Estimate: | Not Specified | ||
| Time Spent: | Not Specified | ||
| Original Estimate: | Not Specified | ||
| Issue Links: |
|
||||||||||||||||||||||||
| Participants: | |||||||||||||||||||||||||
| Description |
|
Observed behavior: The result of multiplying two 32 bit integers is a 32 bit integer, even if there is an overflow.
Consider addition and subtraction overflow as well. For numeric addition and subtraction, adding two 32 bit ints will produce a 32 bit int. Currently division is implemented to always return a double. Also consider for accumulators like $sum. |
| Comments |
| Comment by auto [ 27/Jul/12 ] |
|
Author: {u'date': u'2012-07-26T07:29:12-07:00', u'email': u'dannenberg.matt@gmail.com', u'name': u'Matt Dannenberg'}Message: this was started in the past couple commits, but this covers the final couple cases |
| Comment by auto [ 27/Jul/12 ] |
|
Author: {u'date': u'2012-07-23T14:27:55-07:00', u'email': u'dannenberg.matt@gmail.com', u'name': u'Matt Dannenberg'}Message: also do not count non-numeric types in $avg |
| Comment by auto [ 27/Jul/12 ] |
|
Author: {u'date': u'2012-07-24T06:59:40-07:00', u'email': u'dannenberg.matt@gmail.com', u'name': u'Matt Dannenberg'}Message: also started |
| Comment by Mathias Stearn [ 20/Jul/12 ] |
|
I'm going to use roughly the same rules as $inc updates. If anything is a double output is double. If mix of int and long, output is long. If only int, output is int unless it would overflow, else promote to long. In addition to $sum, this also effects $avg, which absolutely should never overflow since it can lead to misleadingly low results and is always output as a double anyway. |
| Comment by Andy Schwerin [ 20/Jul/12 ] |
|
Would it be better to let users coerce input types to govern output types? |
| Comment by Andy Schwerin [ 20/Jul/12 ] |
|
What is your spec for this? If a user wishes to observe overflow rather than type widening, how will the user express that? |
| Comment by Mathias Stearn [ 19/Jul/12 ] |
|
moving to 2.2-rc1 since this is really easy to fix |
| Comment by Andy Schwerin [ 23/Jun/12 ] |
|
The thing about punting this to 2.3.0 is that I'm concerned that someone will come to depend on the overflow behavior, and we'll be stuck with it. If we get around to allowing aggregation pipelines to output into collections, we'll probably have to support operators to coerce types. |
| Comment by Aaron Staple [ 21/Jun/12 ] |
|
That sounds good to me. I tried to make the initial description pretty general and don't have strong opinions on the specifics. |
| Comment by Andy Schwerin [ 21/Jun/12 ] |
|
I'd be amenable to widening to 64-bit integer, but generally opposed to widening to precision-losing double. |