adriangb commented on code in PR #24236:
URL: https://github.com/apache/datafusion/pull/24236#discussion_r4032460390


##########
datafusion/physical-plan/src/joins/hash_join/inlist_builder.rs:
##########
@@ -74,15 +84,57 @@ pub(super) fn build_struct_inlist_values(
         Arc::new(StructArray::from(arrays_with_fields))
     };
 
-    Ok(Some(source_array))
+    Ok(Some(dedup_inlist_values(source_array)?))
+}
+
+/// Removes duplicate entries from an `IN` list value array, preserving 
first-occurrence order.
+///
+/// Equality is the arrow row-format byte equality produced by 
[`RowConverter`], which is
+/// exactly the equality used elsewhere in DataFusion for grouping. In 
particular NULLs
+/// compare equal to each other, so a list with many NULLs collapses to a 
single NULL. That
+/// is semantics-preserving for `IN`: one NULL in the haystack already yields 
the same
+/// three-valued result as a thousand.
+///
+/// The input is returned unchanged when it holds fewer than two rows, when it 
contains no
+/// duplicates, or when its type cannot be row-encoded (dedup is an 
optimization, never a
+/// requirement).
+fn dedup_inlist_values(values: ArrayRef) -> Result<ArrayRef> {
+    if values.len() < 2 {
+        return Ok(values);
+    }
+
+    let sort_field = SortField::new(values.data_type().clone());
+    if !RowConverter::supports_fields(std::slice::from_ref(&sort_field)) {
+        return Ok(values);
+    }
+
+    let converter = RowConverter::new(vec![sort_field])?;
+    let rows = converter.convert_columns(std::slice::from_ref(&values))?;
+
+    let mut seen: HashSet<Row> = HashSet::with_capacity(values.len());
+    let mut indices: Vec<u32> = Vec::new();
+    for (idx, row) in rows.iter().enumerate() {
+        if seen.insert(row) {
+            indices.push(idx as u32);
+        }
+    }
+
+    if indices.len() == values.len() {
+        // Nothing to remove: skip the copy.
+        return Ok(values);
+    }
+
+    Ok(take(values.as_ref(), &UInt32Array::from(indices), None)?)
 }

Review Comment:
   Wonder if using a sorting deduplication would help here if we're able to 
leave the end list sorted?



-- 
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]

Reply via email to