kosiew commented on code in PR #22386:
URL: https://github.com/apache/datafusion/pull/22386#discussion_r3287252328
##########
datafusion/functions/src/string/common.rs:
##########
@@ -520,6 +520,10 @@ fn case_conversion_utf8view_ascii_inner<F: Fn(&u8) -> u8>(
block_size = block_size.saturating_mul(2);
}
let to_reserve = len.max(block_size as usize);
+ #[expect(
Review Comment:
The rationale on `#[expect(clippy::disallowed_methods)]` mentions that block
sizing prevents capacity overflow, but `Vec::reserve` can still fail due to
allocation failure. If this path is intentionally kept infallible because the
surrounding API cannot return a `Result`, it would help to call that out
explicitly in the reason. Otherwise, it may be worth tracking a follow-up for a
fallible `StringView` builder path using `try_reserve` so allocation failures
can be propagated cleanly.
##########
datafusion/functions/src/strings.rs:
##########
@@ -703,6 +703,10 @@ impl StringViewArrayBuilder {
if self.in_progress.capacity() < required_cap {
self.flush_in_progress();
let to_reserve = (length as usize).max(self.next_block_size() as
usize);
+ #[expect(
Review Comment:
Same thought here on the exemption rationale. The bounded block size helps
avoid overflow issues, but it does not prevent allocation failure. It would be
helpful to clarify that this is an intentional tradeoff for an infallible API
path, or possibly reference a future follow-up for a fallible builder approach.
That would make it clearer that this is a narrow exception rather than a
general safe-use pattern for the lint.
--
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]
