Faster time parsing (~93% faster) #3860
Conversation
tustvold
force-pushed
the
faster-time-parsing
branch
from
3c11b3a
to
aa490c0
Compare
|
I plan to review this carefully tomorrow if no on else had had a chance to do so |
| ("13:00 PM", "%I:%M %P"), | ||
| ("1:00:30.123456 PM", "%I:%M:%S%.f %P"), | ||
| ("1:00:30.123456789 PM", "%I:%M:%S%.f %P"), | ||
| ("1:00:30.123456789123 PM", "%I:%M:%S%.f %P"), |
There was a problem hiding this comment.
Given the experience with #3859, I think it would be valuable to include a time that has 4 and 7 significant digit (aka not a multiple of three)
("1:00:30.1234 PM", "%I:%M:%S%.f %P"),
("1:00:30.123456 PM", "%I:%M:%S%.f %P"),
Perhaps?
| #[test] | ||
| fn test_string_to_time_invalid() { | ||
| let cases = [ | ||
| "25:00", |
There was a problem hiding this comment.
I recommend also testing space and leading colon
" 9:00:00",
":09:00",
```
Perhaps also with a leading T (in case someone splits a RFC timestamp incorrectly)
```
"T9:00:00",
```
There was a problem hiding this comment.
Also, very long subseconds:
("1:00:30.123456789123456789 PM")
There was a problem hiding this comment.
Also intermediate non digits:
"1:00:30.12F456 PM",
arrow-cast/src/parse.rs
Outdated
|
|
||
| _ => &[], | ||
| let (am, bytes) = match bytes.get(bytes.len() - 3..) { | ||
| Some(b" AM") => (Some(true), &bytes[..bytes.len() - 3]), |
There was a problem hiding this comment.
Are Am and Pm valid? If not, perhaps we can add a test showing that
There was a problem hiding this comment.
Turns out this is yet another case of chrono doing something differently from what it is documented to do.... Will fix...
| Some(b" am") => (Some(true), &bytes[..bytes.len() - 3]), | ||
| Some(b" PM") => (Some(false), &bytes[..bytes.len() - 3]), | ||
| Some(b" pm") => (Some(false), &bytes[..bytes.len() - 3]), | ||
| Some(b" AM" | b" am" | b" Am" | b" aM") => { |
Which issue does this PR close?
Closes #3919
Rationale for this change
Faster time parsing
What changes are included in this PR?
Are there any user-facing changes?