alamb commented on code in PR #7074:
URL: https://github.com/apache/arrow-rs/pull/7074#discussion_r1945249812
##########
arrow-cast/src/cast/mod.rs:
##########
@@ -4339,6 +4339,23 @@ mod tests {
}
}
+ #[test]
+ fn test_cast_string_with_large_date_to_date32() {
+ let array = Arc::new(StringArray::from(vec![
+ Some("+10999-12-31"),
+ Some("-0010-02-28"),
Review Comment:
Could you also please add tests for:
```
Some("0010-02-28"),
```
(to show that the negative value actually results in a different parsed
value)
```
"0000-00-00"
"-0000-01-01"
"-0001-01-01"
```
(for boundary cases)
Also a test that shows trying to parse `Some("10999-12-31"),` (more than 4
year digits) correctly results in an error?
##########
arrow-cast/src/cast/mod.rs:
##########
@@ -4339,6 +4339,23 @@ mod tests {
}
}
+ #[test]
+ fn test_cast_string_with_large_date_to_date32() {
+ let array = Arc::new(StringArray::from(vec![
+ Some("+10999-12-31"),
+ Some("-0010-02-28"),
+ ])) as ArrayRef;
+ let to_type = DataType::Date32;
+ let options = CastOptions {
Review Comment:
Without the changes in this PR this code fails like this (so the test does
cover it)
called `Result::unwrap()` on an `Err` value: CastError("Cannot cast string
'+10999-12-31' to value of Date32 type")
##########
arrow-cast/src/parse.rs:
##########
@@ -595,6 +595,26 @@ const EPOCH_DAYS_FROM_CE: i32 = 719_163;
const ERR_NANOSECONDS_NOT_SUPPORTED: &str = "The dates that can be represented
as nanoseconds have to be between 1677-09-21T00:12:44.0 and
2262-04-11T23:47:16.854775804";
fn parse_date(string: &str) -> Option<NaiveDate> {
+ // If the date has an extended (signed) year such as "+10999-12-31" or
"-0012-05-06"
Review Comment:
It was very non clear to me at first that the sign results in different
rules. Can we add some comments to clarify? Something like:
Also, can you include a liml to the reference were you got that sentenece?
```suggestion
// If the date has an extended (signed) year such as "+10999-12-31" or
"-0012-05-06"
//
// According to ISO 8601, years have:
// Four digits or more for the year. Years in the range 0000 to 9999
will be pre-padded by
// zero to ensure four digits. Years outside that range will have a
prefixed positive or negative symbol.
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
