Preserving millisecond precision in DateTime parser - #3934
Conversation
| @@ -1529,6 +1535,7 @@ void DateTime::ConvertTimestampStringToTimePoint(const char* timestamp, DateForm | |||
| if (IsSecondsSinceEpochRepresentable(tt)) | |||
| { | |||
| m_time = std::chrono::system_clock::from_time_t(tt); | |||
| m_time += std::chrono::milliseconds(milliseconds); | |||
There was a problem hiding this comment.
Nit/minor: Time is assigned by comparing to how long it has been since 1970, an edgecase of date before 1970 would render a negative number. if m_time is negative, % 1000 on a negative number stays negative.
Fix: floor when splitting into seconds + ms so the fraction is always 0–999
There was a problem hiding this comment.
not sure if this is a valid case, can you create a string such that a negative value for a millisecond exists? the state machine should take care of that.
| @@ -405,6 +406,7 @@ class DateParser | |||
| bool m_utcAssumed; | |||
| // The size should be at least one byte greater than the maximum possible size so that we could use the last char to indicate the end of the string. | |||
| char m_tz[7]; | |||
| int m_milliseconds; | |||
There was a problem hiding this comment.
nit: i know its inited to zero in the constructor however just make this int m_milliseconds{0}; to avoid issues in the the future. int x does NOT guarantee x will be 0. the constructor value make sure now, but if another constructor is added, this will be a easy miss.
| { | ||
| DateTime gmtDate("2026-09-15T18:00:00.016Z", DateFormat::ISO_8601); | ||
| ASSERT_TRUE(gmtDate.WasParseSuccessful()); | ||
| ASSERT_EQ(16, gmtDate.Millis() % 1000); |
There was a problem hiding this comment.
shouldn't Millis in this context is epoch millis not millis of datetimegmtDate.Millis() be 16? and not gmtDate.Millis() % 1000? 2026-09-15T18:00:00.016Z means that theres .016 seconds on the time meaning 16 miliseconds. shouldnt we not need the modulo operation on it? let me know if im mistaken
286c7ea to
5a80e31
Compare
4 participants
Issue #, if available:
Description of changes:
Added millisecond precision to ISO_8601, ISO_8601 Basic Date Parser, and the AutoDetect path.
Check all that applies:
Check which platforms you have built SDK on to verify the correctness of this PR.
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.