GODRIVER-3594: Add AsFloat64() and AsFloat64OK() conversion functions #2253
Conversation
🧪 Performance ResultsCommit SHA: afe7a03The following benchmark tests for version 6930be63bbc1ac0007e3e70e had statistically significant changes (i.e., |z-score| > 1.96):
For a comprehensive view of all microbenchmark results for this PR's commit, please check out the Evergreen perf task for this patch. |
RafaelCenzano
added
review-priority-normal
API Change Report./v2/bsoncompatible changesRawValue.AsFloat64: added ./v2/x/bsonx/bsoncorecompatible changesValue.AsFloat64: added |
There was a problem hiding this comment.
Pull request overview
This PR adds AsFloat64() and AsFloat64OK() conversion functions to handle BSON-to-float64 conversions, matching existing patterns for integer conversions. It also fixes a bug in the matches.go file where the wrong variable was being checked.
- Implements
AsFloat64()andAsFloat64OK()methods inbsoncore.Valuewith proper error handling for numeric types - Adds wrapper methods in
RawValueto expose the new functionality - Refactors comparison logic to use the new methods instead of manual type checking and casting
- Fixes a bug where
assertionVal.Typewas incorrectly checked instead ofactual.Type
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| x/bsonx/bsoncore/value.go | Implements core AsFloat64() and AsFloat64OK() methods for converting BSON numeric types to float64, and removes redundant variable declarations in AsInt64 methods |
| x/bsonx/bsoncore/value_test.go | Adds comprehensive test cases for the new AsFloat64() and AsFloat64OK() methods covering success cases, error cases, and insufficient bytes scenarios |
| bson/raw_value.go | Adds public wrapper methods AsFloat64() and AsFloat64OK() to expose the core functionality through the RawValue API |
| internal/integration/unified/matches.go | Refactors numeric comparison logic to use new AsFloat64() method and fixes bug where assertionVal.Type was checked instead of actual.Type |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
x/bsonx/bsoncore/value.go
Outdated
| return f64, true | ||
| } | ||
|
|
||
| // Equal compaes v to v2 and returns true if they are equal. |
There was a problem hiding this comment.
Typo in the comment: "compaes" should be "compares".
| // Equal compaes v to v2 and returns true if they are equal. | |
| // Equal compares v to v2 and returns true if they are equal. |
There was a problem hiding this comment.
unrelated to the changes but simple non controversial fix, afe7a03
| // AsFloat64 returns a BSON number as a float64. If the BSON type is not a numeric one, this method | ||
| // will panic. | ||
| func (rv RawValue) AsFloat64() float64 { return convertToCoreValue(rv).AsFloat64() } | ||
|
|
||
| // AsFloat64OK is the same as AsFloat64, except that it returns a boolean instead of | ||
| // panicking. | ||
| func (rv RawValue) AsFloat64OK() (float64, bool) { return convertToCoreValue(rv).AsFloat64OK() } |
There was a problem hiding this comment.
The newly added AsFloat64() and AsFloat64OK() methods lack test coverage in bson/raw_value_test.go. Since other methods like AsInt64() and AsInt64OK() are tested through the underlying bsoncore.Value tests, and this file has existing test coverage, consider adding tests to verify the wrapper methods work correctly.
There was a problem hiding this comment.
@qingyang-hu I will defer to your review on this, it doesn't seem like most of the functions that handle wrapping RawValue conversions are tested and it just converts the raw value then uses the tested AsType functions so in my opinion unless we want to test the raw value conversion for each conversion type it is probably unneeded
1 participant
GODRIVER-3594
Summary
This PR adds
AsFloat64()andAsFloat64OK()functions to handle conversions of BSON values intofloat64valuesBackground & Motivation
Matches functions that exist currently for int conversions. It also allows the removal of using an BSON to int conversion and then a cast to float64. This allows floating point values whereas before values were converted to integers and would lose decimal values.