[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.

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