[SERVER-34975] Server should treat ObjectId timestamp as unsigned Created: 14/May/18 Updated: 06/Dec/22 |
|
| Status: | Backlog |
| Project: | Core Server |
| Component/s: | Aggregation Framework, Querying |
| Affects Version/s: | 3.6.1, 3.7.9 |
| Fix Version/s: | None |
| Type: | Bug | Priority: | Major - P3 |
| Reporter: | Derick Rethans | Assignee: | Backlog - Query Optimization |
| Resolution: | Unresolved | Votes: | 0 |
| Labels: | query-44-grooming | ||
| Remaining Estimate: | Not Specified | ||
| Time Spent: | Not Specified | ||
| Original Estimate: | Not Specified | ||
| Issue Links: |
|
||||||
| Assigned Teams: |
Query Optimization
|
||||||
| Operating System: | ALL | ||||||
| Backport Requested: |
v4.0
|
||||||
| Steps To Reproduce: | On the shell, where it correctly uses the unsigned 32-bit integer value:
In aggregation, where it incorrectly uses the timestamp as a signed 32-bit integer:
|
||||||
| Sprint: | Query 2018-06-04, Query 2018-07-02 | ||||||
| Participants: | |||||||
| Description |
|
The ObjectId type in the server treats the timestamp portion as a 32-bit signed integer. This should be treated as an unsigned integer instead, increasing the range of dates into the future, and bringing the server more in line with the drivers. As one example of what can go wrong, see below. When the $dateToString function is applied to an ObjectID's timestamp portion, the timestamp portion is interpreted as a 32-bit signed integer. This is inconsistent with the shell's ObjectId.getTimestamp() method, and the definition of the first 4 bytes of the https://docs.mongodb.com/manual/reference/method/ObjectId/: "a 4-byte value representing the seconds since the Unix epoch" |
| Comments |
| Comment by Charlie Swanson [ 31/Jul/18 ] |
|
I think derick makes a strong point. While I don't think it's urgent we fix this issue, I think the fact that the server is not inline with the spec is indeed a bug that should not be closed as "Won't Fix". I re-opened the ticket, adjusted the title to make it more clear what the issue is, and put it back into Needs Triage. |
| Comment by Derick Rethans [ 24/Jul/18 ] |
|
I would like to register my objection about this being closed. This bug was introduced as part of the work on the date/time support in Aggregation that I did work on last year, and I must inadvertently have introduced this bug. The time portion of the ObjectID "object" should be interpreted as a 32-bit unsigned integer as per: https://github.com/mongodb/specifications/blob/master/source/objectid.rst#timestamp-field — I realise that spec is written specifically for drivers but that doesn't mean the server should handle it wrongly. |
| Comment by Justin Seyster [ 28/Jun/18 ] |
|
I forgot to include an explanation in the commit message, so I'll leave one here. Changing the type of the Timestamp typedef also changed the behavior of OID::fromTerm() function, which creates an OID with a time component based on std::numeric_limits<Timestamp>::max(). We could have tweaked this to solve the problems, but this behavior is subtle enough that we decided to simply revert the change. |
| Comment by Githook User [ 28/Jun/18 ] |
|
Author: {'username': 'jseyster', 'name': 'Justin Seyster', 'email': 'justin.seyster@mongodb.com'}Message: Revert "SERVER-34975 Treat an OID-embedded timestamp as unsigned." This reverts commit a9deb701a7533b566a2eb703aff70bf2f03b147c. |
| Comment by Githook User [ 22/May/18 ] |
|
Author: {'username': 'jseyster', 'name': 'Justin Seyster', 'email': 'justin.seyster@mongodb.com'}Message: SERVER-34975 Treat an OID-embedded timestamp as unsigned. |
| Comment by Asya Kamsky [ 19/May/18 ] |
|
Apparently this code has been like this for over four years, but I think it was not exposed/used anywhere except this test (which only checked "now): https://github.com/mongodb/mongo/blob/master/src/mongo/dbtests/jsobjtests.cpp#L1397-L1412 (that is until |