U0001F3A2 commented on issue #23640:
URL: https://github.com/apache/datafusion/issues/23640#issuecomment-5002048618
Case 3 is worse than being read as an empty format string. It can silently
return a wrong value instead of erroring.
`''` is only what a null slot happens to contain when the array is built the
usual way. Arrow permits a null slot to keep its backing bytes, and
`handle_array_op` reads the format with `.value(pos)`
(`datafusion/functions/src/datetime/common.rs:487-489`) without checking
validity, so it parses whatever is physically there.
Reproduced on f151c10fd with a format array whose slot 0 is NULL but still
holds `%d/%m/%Y`:
```rust
let formats = GenericStringArray::<i32>::from(vec!["%d/%m/%Y", "%Y-%m-%d"]);
let data = formats
.to_data()
.into_builder()
.null_bit_buffer(Some(Buffer::from([0b00000010u8]))) // slot 0 NULL,
slot 1 valid
.build()
.unwrap();
let formats = GenericStringArray::<i32>::from(data);
assert!(formats.is_null(0));
assert_eq!(formats.value(0), "%d/%m/%Y"); // null, but the bytes are still
there
// to_date(['08/09/2020', '2020-09-08'], formats)
```
Row 0's format is NULL, so the row should be NULL. It returns `2020-09-08`,
parsed under the retained `%d/%m/%Y`.
The same `is_null(pos)` check covers both, so the fix doesn't change. Worth
tagging this as a correctness bug rather than only a spurious error. PR shortly.
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
