kosiew commented on code in PR #20787:
URL: https://github.com/apache/datafusion/pull/20787#discussion_r2972910775
##########
datafusion/functions/src/strings.rs:
##########
@@ -165,6 +187,12 @@ impl StringViewArrayBuilder {
self.block.push_str(array.value(i));
}
}
+ ColumnarValueRef::NullableBinaryArray(array) => {
Review Comment:
Nice catch fixing validation in the `StringArrayBuilder` and
`LargeStringArrayBuilder` finish paths.
I think there is still one case left here though.
`concat` still prefers `Utf8View` whenever any argument has that type. In
that branch, binary inputs are written through `StringViewArrayBuilder`, and
both binary array variants still call `std::str::from_utf8(...).unwrap()` on
the bytes. That means a query like `concat(arrow_cast('ok', 'Utf8View'),
binary_col)` can still panic on malformed UTF-8 instead of returning an
execution error.
The new tests cover valid `Utf8View + Binary` inputs and invalid `Binary +
Binary` inputs, but they do not cover the invalid mixed `Utf8View + Binary`
case that still reaches this unchecked path.
--
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]
