[GODRIVER-2046] Return clearer error when truncating a float64 to float32 without the truncate tag Created: 14/Jun/21 Updated: 02/Nov/23 Resolved: 01/Nov/23 |
|
| Status: | Closed |
| Project: | Go Driver |
| Component/s: | BSON |
| Affects Version/s: | 1.4.7, 1.5.1, 1.5.2, 1.5.3 |
| Fix Version/s: | None |
| Type: | Improvement | Priority: | Minor - P4 |
| Reporter: | Matt Hartstonge | Assignee: | Unassigned |
| Resolution: | Done | Votes: | 1 |
| Labels: | None | ||
| Remaining Estimate: | Not Specified | ||
| Time Spent: | Not Specified | ||
| Original Estimate: | Not Specified | ||
| Environment: |
Windows |
||
| Documentation Changes Summary: | 1. What would you like to communicate to the user about this feature? |
| Description |
|
Motivation If attempting to unmarshal a float64 into a float32 results in truncation, and the destination does not have the truncate tag, the error errCannotTruncate is returned. This error consists of the message "float64 can only be truncated to an integer type when truncation is enabled". The error message is misleading. A float64 can be unmarshaled into a float32 if the truncate tag is present. Scope Original ticket description We've run across an issue where the truncation logic in the BSON lib is causing an issue due to a floating point rounding error when trying to validate a float32 value when comparing float64(float32(f)) to the mongo returned float64(f). Here's the minimum reproducible showcasing that 0.1 returns incorrectly: https://play.golang.org/p/eYANaXFKVk9
Returns: This is occurring here: https://github.com/mongodb/mongo-go-driver/blob/49af6e279ffe534210dca95e375d372517145e44/bson/bsoncodec/default_value_decoders.go#L480 |
| Comments |
| Comment by Githook User [ 02/Nov/23 ] | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Author: {'name': 'Steven Silvester', 'email': 'steven.silvester@ieee.org', 'username': 'blink1073'}Message: Co-authored-by: Ernie Hershey <ernie.hershey@mongodb.com> | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Comment by Matt Hartstonge [ 22/Jun/21 ] | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Yeah, that would be helpful. 👍 I bet it's down to a difference in how `mgo` was pushing in `float32`s which has thrown off all our data :/ Thanks for your help Kevin, has been great 😀 | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Comment by Kevin Albertson [ 22/Jun/21 ] | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
matt@linc-ed.com that seems like a good solution! The default decoding behavior does give users an extra safety check to prevent them from inadvertently truncating values, so I do not think we will change the default behavior. Is your solution satisfactory? If so, I will proceed to update the ticket description to return a clearer error when truncating a float64 to float32 without the truncate tag. | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Comment by Matt Hartstonge [ 16/Jun/21 ] | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Hey Kevin, Yeah, setting the BSON tag works. 👍 but ... We generate all our microservices directly from protoc-gen-go which doesn't currently have a mechanism (natively) to enable tag addition/mutation. We encode our database models directly into the proto files due to the generation and API interface guarantees it provides for API consumers. Our workaround has been to copy the default decoder, register it and return the {{float32}}s, our mongo connection logic is all wrapped in a common library so it's been extremely quick to patch it out this way across all our services:
| |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Comment by Kevin Albertson [ 16/Jun/21 ] | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Great thank you for the additional tests matt@linc-ed.com! There is a struct tag that is documented as having the intent to allow truncation of doubles into a float32 with loss of precision: Modifying the example I included, that can be updated like so:
The error message does seem misleading. If the truncate tag resolves your issue, I will repurpose this ticket to consider updating the error returned when truncating a float64 to float32 without the truncate tag. | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Comment by Matt Hartstonge [ 15/Jun/21 ] | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Here's an extended table test with some numbers that generally throw floating point conversion errors:
| |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Comment by Matt Hartstonge [ 15/Jun/21 ] | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Hi @kevin, That seems to be the bson repro! Nice!
As an aside a colleague pointed out - the error returned talks about being truncated to an integer type, whereas this error is returned when dealing with floats. I'm guessing that this would cause a breaking API change though. | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Comment by Kevin Albertson [ 15/Jun/21 ] | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Hi matt@linc-ed.com, thank you for the report! To check, is the behavior described occurring when trying to unmarshal a BSON double into a struct containing a float32 field? Would the following be a reproducing case?
|