[SERVER-37450] Value::toString() string conversion for input double needs improvement Created: 03/Oct/18  Updated: 06/Dec/22

Status: Backlog
Project: Core Server
Component/s: Aggregation Framework
Affects Version/s: None
Fix Version/s: None

Type: Improvement Priority: Minor - P4
Reporter: Patrick Meredith Assignee: Backlog - Query Execution
Resolution: Unresolved Votes: 0
Labels: None
Remaining Estimate: Not Specified
Time Spent: Not Specified
Original Estimate: Not Specified

Assigned Teams:
Query Execution
Participants:

 Description   

Currently, converting doubles to strings produces strings no longer than 6 characters, and drops anything after the decimal place that would require going above 6 characters. At 6 decimal places it automatically converts to scientific notation. This is an issue resulting from

ostream <<

which can be seen here:
https://repl.it/repls/DefensiveCoarseMedia

The easiest way to improve this is to change the operator << definition for bson docs here:
https://github.com/mongodb/mongo/blob/b89f752fe24dd4b24d59f39ea2f45b01aaca8250/src/mongo/db/pipeline/value.cpp#L1115

Since this appears to be a problem even using std::setprecision (it sets the number of characters in the string instead of the actual precision), I would suggest using std::sprintf and std::snprintf like the following:

  auto input = 500000000000000.1;
  auto size = std::snprintf(NULL, 0, "%.10f", input);
  auto buff = static_cast<char *>(alloca(sizeof(char) * size));
  std::sprintf(buff, "%.10f", input);
  out << buff;

If we decide we want to use 10 decimal places.



 Comments   
Comment by Bruce Lucas (Inactive) [ 26/Nov/18 ]

It seems reasonable that the conversion should round-trip, i.e. the conversion to string and back to double should give you back the same number. There's a constant maxdigits10 that defines the number of digits of precision required to do this (it's 17 for double). That page also implies that setprecision(maxdigits10) should cause the << operator to behave this way.

Comment by Asya Kamsky [ 26/Nov/18 ]

Here is the actual original ticket that I knew I filed in 2014 SERVER-12970 - it was closed as "won't fix" early this year I think because it was perceived that the type conversion work eliminated this problem. Perhaps it was closed incorrectly?

Comment by Asya Kamsky [ 26/Nov/18 ]

david.storch that's only the case for some inputs

{ "a" : 5000.1, "aType" : "double", "aAsString" : "5000.1" }
{ "a" : 50001.2, "aType" : "double", "aAsString" : "50001.2" }
{ "a" : 500012.3, "aType" : "double", "aAsString" : "500012" }
{ "a" : 5000123.4, "aType" : "double", "aAsString" : "5.00012e+06" }
{ "a" : 50001234.5, "aType" : "double", "aAsString" : "5.00012e+07" }

Comment by Asya Kamsky [ 15/Nov/18 ]

This looks like duplicate of SERVER-23204

Comment by Charlie Swanson [ 26/Oct/18 ]

asya to investigate if there is a duplicate ticket. Please put it back into Needs Triage with your findings.

Comment by David Storch [ 03/Oct/18 ]

Gotcha. As a concrete example:

db.c.aggregate([{$project: {out: {$convert: {input: 500000.1, to: "string"}}}}])

results in the output string "500000" rather than "500000.1".

Comment by Patrick Meredith [ 03/Oct/18 ]

david.storch Yes, it has user-facing impact when issuing `$convert` from a double value to a string.

Comment by David Storch [ 03/Oct/18 ]

patrick.meredith, do I understand correctly that you are specifically talking about the implementation of Value::toString(), when the Value represents a NumberDouble? Does this have any user-facing impact?

Comment by Patrick Meredith [ 03/Oct/18 ]

Another option is to forgo the first snprintf that figures out the size and just use some max length like 100 (and use snprintf for format). Alloca'ing 100 bytes takes the same time as alloca'ing n bytes, so this might end up being faster.:

 auto buff = static_cast<char *>(alloca(100 * sizeof(char)));
 std::snprintf(buff, 100, "%.10f", 50000000000.00001);
 out << buff;   

Generated at Thu Feb 08 04:46:01 UTC 2024 using Jira 9.7.1#970001-sha1:2222b88b221c4928ef0de3161136cc90c8356a66.