-
Notifications
You must be signed in to change notification settings - Fork 9
Typo in Approximate UUID timestamp calculations #131
Comments
Would it be better to put: |
I don't think so. After all, if you divide a microsecond by 1000, you get a nanosecond. |
I thought you multiply a Microsecond to get a Nanosecond? Else, the last part of the sentence is also wrong...
But that aside, my point in adding both was you could theoretically convert 1µs or 2µs+ by 1000 or 1024 to get some form of MS/NS. |
Is it correct to use a number of nanoseconds in UUID v7? (Obviously, that would require +20 bits in timestamp section) |
It is not correct (see 6.2. Monotonicity and Counters):
The accuracy of the timestamp has been limited on purpose after much discussion: #23 (comment) 6.1. Timestamp Considerations:
Also, it's pointless to make the timestamp more accurate (nanoseconds) than the time source (microseconds) |
My 2c here is that we should either leave the text how it is (just because we're very close to getting this thing finally published), or if we change anything my suggestion would be to go back to not having that text about dividing by 1024 at all. The spec already says there are no guarantees about the accuracy of the timestamp, and it's up to the implementation to determine how far off from the original value they want to make it. Dividing by 1024 gives a decidedly "incorrect" value (off by months if not years depending on the timestamp), but since there's no guarantee, it's still technically allowed. But I do not think we should encourage it. Anyway, I just don't think trying to carefully fiddle with that text to take other edge cases into account is worth holding up the document for. |
@sergeyprokhorenko Just so we understand, what was the intention of the original question - what problem were you trying to solve with this:
|
You haven't given any convincing arguments. We still have time to correct the error in the text. The time source gives us the number of microseconds. To get the number of milliseconds for UUIDv7, they need to be divided by 1000. But the approximate division by 1024 takes much less computational resources. There is no point in exact division by 1000. This is a problem that absolutely all implementations face. Therefore, we cannot ignore it, as you suggest. Why it was proposed to divide some one microsecond by 1024, I cannot explain, because this is an erroneous phrase. |
@sergeyprokhorenko Thanks and understood. Until that last message I didn't get what error in the text was that you're referring to (you suggested a change but didn't clearly say why). But okay, I think I get it now: Your concern is that dividing microseconds by 1024 doesn't yield milliseconds, it yields . Is that correct? If so, I guess we could change the sequence of that text to make it read more correctly like so. As suggested text to give the whole thing:
If we're going to leave in this 1024 text, then that would be my suggestion (assuming I understand the concern correctly.) That said, I still don't understand how suggesting the user divide by 1024 is helpful at all. The rest of the specification clearly says milliseconds (and changing text above here doesn't change that), so if you're dividing by 1024 you're ignoring the rest of the spec. And, that's okay, it's allowed as mentioned earlier because there are no guarantees on the accuracy of the timestamp. But I still don't see any reason to suggest to implementors to do this 1024 division since it's clearly not what anything else in the document says to do. So I'm not going to die over it, but it really seems like it would be better to just take this 1024 text out entirely. You can still do whatever you want, not having this text doesn't prevent that, but it would prevent the kind of confusion generated as we can see in this discussion. Everyone who reads this and does the math is going to go "uhm, if we divide by 1024 then it's not milliseconds, why is it telling me to do that?" |
Your new wording is perfect:
I agree with you that the spec should be internally consistent. Therefore, I propose to expand the wording:
It's only important that it's not days or microseconds: #23 (comment) |
It is typo. Should be dividing |
I revised the doc with @bradleypeabody's suggestion: I have not made the second change for "or time intervals close to milliseconds" will let the discussion here play out. |
@sergeyprokhorenko Thanks for the feedback. I would like to leave it at this (with the most recent change above that Kyzer did) since we've addressed the original concern. As a general concept, I'd like to just leave it saying "milliseconds" wherever possible (i.e. let's not introduce this other "or time intervals close to milliseconds"). The spec is specifically milliseconds, and think we've sufficiently covered the idea that if you put something else in there that isn't exactly milliseconds then the time will be off if it's parsed by some other UUID parser, but that's a totally acceptable tradeoff for implementations that don't care about timestamp interoperability. IMO we've covered it well enough at this point. |
@kyzer-davis,
Is it possible to change
"such as dividing a microsecond by 1024 (or some other value) instead of 1000 to obtain a millisecond value"
for
"such as dividing a number of microseconds by 1024 (or some other value) instead of 1000 to obtain a millisecond value"?
The text was updated successfully, but these errors were encountered: