Explicitly don't support leap seconds for Time32/Time64 arrays when parsing
- Dominant language
- Rust
- Stars
- 3.6k
- Forks
- 1.3k
- Avg merge
- 2d 16h
- Merged PRs (30d)
- 168
Description
**Describe the bug**
Arrow spec states that leap seconds should not be supported:
https://github.com/apache/arrow/blob/13b22346d36b9952df5c988c9425b9e5bc4f09c4/format/Schema.fbs#L265-L274
> The allowed values are between 0 (inclusive) and 86400 (=24\*60*60) seconds
(exclusive), adjusted for the time unit (for example, up to 86400000
exclusive for the MILLISECOND unit).
This definition doesn't allow for leap seconds. Time values from
measurements with leap seconds will need to be corrected when ingesting
into Arrow (for example by replacing the value 86400 with 86399).
However we introduced valid parsing of leap seconds in this PR: https://github.com/apache/arrow-rs/pull/3101
**To Reproduce**
See this test:
https://github.com/apache/arrow-rs/blob/8fff5e4a075c20619690c967e217163cd74e0656/arrow-cast/src/parse.rs#L1759-L1760
**Expected behavior**
According to Arrow spec above, we should convert to `86399` value instead?
Either that, or simply update docs of Time32/Time64 datatype & parsing to indicate the current behaviour.
**Additional context**
An interesting thing to note is that whilst parsing into the integer representation is successful, when trying to display the value, it doesn't actually display properly anyway. See below example:
```rust
#[test]
fn test_pretty_format_time_32_second_invalid() {
let array: Time32SecondArray = vec![
Some(86_400),
]
.into();
println!("{:?}", array);
let schema = Arc::new(Schema::new(vec![Field::new(
"f",
array.data_type().clone(),
true,
)]));
let batch = RecordBatch::try_new(schema, vec![Arc::new(array)]).unwrap();
let table = pretty_format_batches(&[batch]).unwrap().to_string();
println!("{}", table);
}
```
Output:
```
PrimitiveArray
[
null,
]
+---------------------------------------------------------------------------+
| f |
+---------------------------------------------------------------------------+
| ERROR: Cast error: Failed to convert 86400 to temporal for Time32(Second) |
+---------------------------------------------------------------------------+
```
Contributor guide
Research direction
Start with the leap-second parsing test in arrow-cast/src/parse.rs referenced by the issue, then compare its behavior with the Arrow Schema.fbs specification. Check the displayed Time32(Second) output from the provided pretty-format example and establish whether parsing should reject or correct 86400; done means the chosen behavior and corresponding documentation or tests are consistent.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- rust
- Domain
- data
- Issue type
- Bug
- Difficulty
- 3/5
- Estimated time
- 1-2 days
- Activity status
- Stale
- Clarity
- Mostly clear
- Newbie friendliness
- 35/100