[SERVER-39033] swap out int and long long for int32_t and int64_t in BSONElement Created: 16/Jan/19 Updated: 06/Dec/22 Resolved: 02/Jul/20 |
|
| Status: | Closed |
| Project: | Core Server |
| Component/s: | Internal Code |
| Affects Version/s: | None |
| Fix Version/s: | None |
| Type: | Improvement | Priority: | Major - P3 |
| Reporter: | Eric Milkie | Assignee: | Backlog - Storage Execution Team |
| Resolution: | Won't Do | Votes: | 0 |
| Labels: | None | ||
| Remaining Estimate: | Not Specified | ||
| Time Spent: | Not Specified | ||
| Original Estimate: | Not Specified | ||
| Assigned Teams: |
Storage Execution
|
| Participants: |
| Description |
|
It would be helpful to change BSONElement and friends (BSONObjBuilder, etc) to use explicitly-sized types for numbers. Right now I frequently have to slog through casting between long long and std::uint64_t when working with BSON, and it would be nice to have things work more seamlessly. |
| Comments |
| Comment by Eric Milkie [ 16/Jan/19 ] |
|
Right, I don't want a BSONObjBuilder::append() function that takes a std::uint64_t to append a NumberLong. But changing BSONObjBuilder::appendTimestamp() to take a std::uint64_t instead of an unsigned long long is very desirable. And changing the append() function that takes a long long to instead take a std::int64_t would also be desirable here. Basically, swapping out only exact equivalent types. |
| Comment by Andrew Morrow (Inactive) [ 16/Jan/19 ] |
|
Overall, though, I think we agree. I'd like the types that the builder offers to be more exact with respect to both what the BSON standard offers and the dedicated fixed-width C++ types, have fewer overloads, and require callers to be explicit. |
| Comment by Andrew Morrow (Inactive) [ 16/Jan/19 ] |
|
Ah, I was noticing your complaint about unsigned 64-bit types in particular. I think you should need to cast there, because of the unsigned-ness. I don't want to paper over that for callers. A boxed type meaning a dedicated structure for each type: https://github.com/mongodb/mongo-cxx-driver/blob/master/src/bsoncxx/types.hpp#L82-L105 |
| Comment by Eric Milkie [ 16/Jan/19 ] |
|
I think my description was a bit unclear, as I do not want any more overloads (hence "swap out" in the title). Specifically, the BSON spec says "int32" and "int64", so I think we should have BSONElement::numberInt() and BSONElement::numberLong() return std::int32_t and std::int64_t. I'm not sure what a boxed type is. |
| Comment by Andrew Morrow (Inactive) [ 16/Jan/19 ] |
|
I'd argue we should go the other way, and reduce the space of types the builder accepts: the builder API should offer only exactly the types that the BSON spec declares to be supported. Only the caller can know for sure what the correct interpretation/conversion should be for other types, so that should be decided at the call site. Adding more overloads I think just increases the risk of subtle errors. The C++ driver takes this approach, and even goes a little further: its BSON builder only accepts boxed BSON types like b_int, b_double, etc. |