Dandandan commented on code in PR #12969:
URL: https://github.com/apache/datafusion/pull/12969#discussion_r1806040566
##########
datafusion/physical-plan/src/joins/utils.rs:
##########
@@ -1361,17 +1361,47 @@ pub(crate) fn append_right_indices(
if right_unmatched_indices.is_empty() {
Ok((left_indices, right_indices))
} else {
+ // `into_builder()` can fail here when there is nothing to be
filtered and
+ // left_indices or right_indices has the same reference to the
cached indices.
+ // In that case, we use a slower alternative.
+
// the new left indices: left_indices + null array
- let Ok(mut new_left_indices_builder) = left_indices.into_builder()
else {
- return internal_err!("Failed to convert left indices to
builder");
- };
+ let mut new_left_indices_builder =
+ left_indices.into_builder().unwrap_or_else(|left_indices| {
+ let mut builder = UInt64Builder::with_capacity(
+ left_indices.len() + right_unmatched_indices.len(),
+ );
+ debug_assert_eq!(
+ left_indices.null_count(),
+ 0,
+ "expected left indices to have no nulls"
+ );
+ builder.append_slice(left_indices.values());
Review Comment:
I wonder if at this point the left array `Arc` can be dropped such that the
right side can be converted to builder without issue.
--
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]
