You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@datafusion.apache.org by "brancz (via GitHub)" <gi...@apache.org> on 2026/01/14 14:52:20 UTC
[PR] datafusion/common: Add support for hashing ListView arrays [datafusion]
brancz opened a new pull request, #19814:
URL: https://github.com/apache/datafusion/pull/19814
## Which issue does this PR close?
Implements the hashing part of https://github.com/apache/datafusion/issues/19782
## Rationale for this change
To aggregate by ListView arrays, we need to be able to hash them.
## What changes are included in this PR?
The hashing code and tests.
## Are these changes tested?
Yes, unit tests.
## Are there any user-facing changes?
No, only additive functionality.
@alamb @Jefffrey
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] datafusion/common: Add support for hashing ListView arrays [datafusion]
Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb merged PR #19814:
URL: https://github.com/apache/datafusion/pull/19814
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] datafusion/common: Add support for hashing ListView arrays [datafusion]
Posted by "Jefffrey (via GitHub)" <gi...@apache.org>.
Jefffrey commented on code in PR #19814:
URL: https://github.com/apache/datafusion/pull/19814#discussion_r2690854826
##########
datafusion/common/src/hash_utils.rs:
##########
@@ -1128,6 +1176,100 @@ mod tests {
assert_eq!(hashes[1], hashes[6]); // null vs empty list
}
+ #[test]
+ // Tests actual values of hashes, which are different if forcing collisions
+ #[cfg(not(feature = "force_hash_collisions"))]
+ fn create_hashes_for_list_view_arrays() {
+ use arrow::buffer::{NullBuffer, ScalarBuffer};
+
+ // Create values array: [0, 1, 2, 3, null, 5]
+ let values = Arc::new(Int32Array::from(vec![
+ Some(0),
+ Some(1),
+ Some(2),
+ Some(3),
+ None,
+ Some(5),
+ ])) as ArrayRef;
+ let field = Arc::new(Field::new("item", DataType::Int32, true));
+
+ // Create ListView with the following logical structure:
+ // Row 0: [0, 1, 2] (offset=0, size=3)
+ // Row 1: null (null bit set)
+ // Row 2: [3, null, 5] (offset=3, size=3)
+ // Row 3: [3, null, 5] (offset=3, size=3) - same as row 2
+ // Row 4: null (null bit set)
+ // Row 5: [0, 1, 2] (offset=0, size=3) - same as row 0
+ // Row 6: [] (offset=0, size=0) - empty list
+ let offsets = ScalarBuffer::from(vec![0i32, 0, 3, 3, 0, 0, 0]);
+ let sizes = ScalarBuffer::from(vec![3i32, 0, 3, 3, 0, 3, 0]);
+ let nulls = Some(NullBuffer::from(vec![true, false, true, true, false, true, true]));
+
+ let list_view_array = Arc::new(ListViewArray::new(
+ field,
+ offsets,
+ sizes,
+ values,
+ nulls,
+ )) as ArrayRef;
+
+ let random_state = RandomState::with_seeds(0, 0, 0, 0);
+ let mut hashes = vec![0; list_view_array.len()];
+ create_hashes(&[list_view_array], &random_state, &mut hashes).unwrap();
+
+ assert_eq!(hashes[0], hashes[5]); // same content [0, 1, 2]
+ assert_eq!(hashes[1], hashes[4]); // both null
+ assert_eq!(hashes[2], hashes[3]); // same content [3, null, 5]
+ assert_eq!(hashes[1], hashes[6]); // null vs empty list
Review Comment:
Perhaps some negative tests to ensure hashes differ
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] datafusion/common: Add support for hashing ListView arrays [datafusion]
Posted by "Jefffrey (via GitHub)" <gi...@apache.org>.
Jefffrey commented on code in PR #19814:
URL: https://github.com/apache/datafusion/pull/19814#discussion_r2694145138
##########
datafusion/common/src/hash_utils.rs:
##########
@@ -1128,6 +1176,100 @@ mod tests {
assert_eq!(hashes[1], hashes[6]); // null vs empty list
}
+ #[test]
+ // Tests actual values of hashes, which are different if forcing collisions
+ #[cfg(not(feature = "force_hash_collisions"))]
+ fn create_hashes_for_list_view_arrays() {
+ use arrow::buffer::{NullBuffer, ScalarBuffer};
+
+ // Create values array: [0, 1, 2, 3, null, 5]
+ let values = Arc::new(Int32Array::from(vec![
+ Some(0),
+ Some(1),
+ Some(2),
+ Some(3),
+ None,
+ Some(5),
+ ])) as ArrayRef;
+ let field = Arc::new(Field::new("item", DataType::Int32, true));
+
+ // Create ListView with the following logical structure:
+ // Row 0: [0, 1, 2] (offset=0, size=3)
+ // Row 1: null (null bit set)
+ // Row 2: [3, null, 5] (offset=3, size=3)
+ // Row 3: [3, null, 5] (offset=3, size=3) - same as row 2
+ // Row 4: null (null bit set)
+ // Row 5: [0, 1, 2] (offset=0, size=3) - same as row 0
+ // Row 6: [] (offset=0, size=0) - empty list
+ let offsets = ScalarBuffer::from(vec![0i32, 0, 3, 3, 0, 0, 0]);
+ let sizes = ScalarBuffer::from(vec![3i32, 0, 3, 3, 0, 3, 0]);
+ let nulls = Some(NullBuffer::from(vec![true, false, true, true, false, true, true]));
+
+ let list_view_array = Arc::new(ListViewArray::new(
+ field,
+ offsets,
+ sizes,
+ values,
+ nulls,
+ )) as ArrayRef;
+
+ let random_state = RandomState::with_seeds(0, 0, 0, 0);
+ let mut hashes = vec![0; list_view_array.len()];
+ create_hashes(&[list_view_array], &random_state, &mut hashes).unwrap();
+
+ assert_eq!(hashes[0], hashes[5]); // same content [0, 1, 2]
+ assert_eq!(hashes[1], hashes[4]); // both null
+ assert_eq!(hashes[2], hashes[3]); // same content [3, null, 5]
+ assert_eq!(hashes[1], hashes[6]); // null vs empty list
Review Comment:
Yep
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] datafusion/common: Add support for hashing ListView arrays [datafusion]
Posted by "brancz (via GitHub)" <gi...@apache.org>.
brancz commented on code in PR #19814:
URL: https://github.com/apache/datafusion/pull/19814#discussion_r2693501390
##########
datafusion/common/src/hash_utils.rs:
##########
@@ -1128,6 +1176,100 @@ mod tests {
assert_eq!(hashes[1], hashes[6]); // null vs empty list
}
+ #[test]
+ // Tests actual values of hashes, which are different if forcing collisions
+ #[cfg(not(feature = "force_hash_collisions"))]
+ fn create_hashes_for_list_view_arrays() {
+ use arrow::buffer::{NullBuffer, ScalarBuffer};
+
+ // Create values array: [0, 1, 2, 3, null, 5]
+ let values = Arc::new(Int32Array::from(vec![
+ Some(0),
+ Some(1),
+ Some(2),
+ Some(3),
+ None,
+ Some(5),
+ ])) as ArrayRef;
+ let field = Arc::new(Field::new("item", DataType::Int32, true));
+
+ // Create ListView with the following logical structure:
+ // Row 0: [0, 1, 2] (offset=0, size=3)
+ // Row 1: null (null bit set)
+ // Row 2: [3, null, 5] (offset=3, size=3)
+ // Row 3: [3, null, 5] (offset=3, size=3) - same as row 2
+ // Row 4: null (null bit set)
+ // Row 5: [0, 1, 2] (offset=0, size=3) - same as row 0
+ // Row 6: [] (offset=0, size=0) - empty list
+ let offsets = ScalarBuffer::from(vec![0i32, 0, 3, 3, 0, 0, 0]);
+ let sizes = ScalarBuffer::from(vec![3i32, 0, 3, 3, 0, 3, 0]);
+ let nulls = Some(NullBuffer::from(vec![true, false, true, true, false, true, true]));
+
+ let list_view_array = Arc::new(ListViewArray::new(
+ field,
+ offsets,
+ sizes,
+ values,
+ nulls,
+ )) as ArrayRef;
+
+ let random_state = RandomState::with_seeds(0, 0, 0, 0);
+ let mut hashes = vec![0; list_view_array.len()];
+ create_hashes(&[list_view_array], &random_state, &mut hashes).unwrap();
+
+ assert_eq!(hashes[0], hashes[5]); // same content [0, 1, 2]
+ assert_eq!(hashes[1], hashes[4]); // both null
+ assert_eq!(hashes[2], hashes[3]); // same content [3, null, 5]
+ assert_eq!(hashes[1], hashes[6]); // null vs empty list
Review Comment:
can you be more specific what you mean? are you just asking to test that eg. hash 0 and 2 are different?
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] datafusion/common: Add support for hashing ListView arrays [datafusion]
Posted by "brancz (via GitHub)" <gi...@apache.org>.
brancz commented on code in PR #19814:
URL: https://github.com/apache/datafusion/pull/19814#discussion_r2694229588
##########
datafusion/common/src/hash_utils.rs:
##########
@@ -1128,6 +1176,100 @@ mod tests {
assert_eq!(hashes[1], hashes[6]); // null vs empty list
}
+ #[test]
+ // Tests actual values of hashes, which are different if forcing collisions
+ #[cfg(not(feature = "force_hash_collisions"))]
+ fn create_hashes_for_list_view_arrays() {
+ use arrow::buffer::{NullBuffer, ScalarBuffer};
+
+ // Create values array: [0, 1, 2, 3, null, 5]
+ let values = Arc::new(Int32Array::from(vec![
+ Some(0),
+ Some(1),
+ Some(2),
+ Some(3),
+ None,
+ Some(5),
+ ])) as ArrayRef;
+ let field = Arc::new(Field::new("item", DataType::Int32, true));
+
+ // Create ListView with the following logical structure:
+ // Row 0: [0, 1, 2] (offset=0, size=3)
+ // Row 1: null (null bit set)
+ // Row 2: [3, null, 5] (offset=3, size=3)
+ // Row 3: [3, null, 5] (offset=3, size=3) - same as row 2
+ // Row 4: null (null bit set)
+ // Row 5: [0, 1, 2] (offset=0, size=3) - same as row 0
+ // Row 6: [] (offset=0, size=0) - empty list
+ let offsets = ScalarBuffer::from(vec![0i32, 0, 3, 3, 0, 0, 0]);
+ let sizes = ScalarBuffer::from(vec![3i32, 0, 3, 3, 0, 3, 0]);
+ let nulls = Some(NullBuffer::from(vec![true, false, true, true, false, true, true]));
+
+ let list_view_array = Arc::new(ListViewArray::new(
+ field,
+ offsets,
+ sizes,
+ values,
+ nulls,
+ )) as ArrayRef;
+
+ let random_state = RandomState::with_seeds(0, 0, 0, 0);
+ let mut hashes = vec![0; list_view_array.len()];
+ create_hashes(&[list_view_array], &random_state, &mut hashes).unwrap();
+
+ assert_eq!(hashes[0], hashes[5]); // same content [0, 1, 2]
+ assert_eq!(hashes[1], hashes[4]); // both null
+ assert_eq!(hashes[2], hashes[3]); // same content [3, null, 5]
+ assert_eq!(hashes[1], hashes[6]); // null vs empty list
Review Comment:
Done
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] datafusion/common: Add support for hashing ListView arrays [datafusion]
Posted by "brancz (via GitHub)" <gi...@apache.org>.
brancz commented on PR #19814:
URL: https://github.com/apache/datafusion/pull/19814#issuecomment-3758975561
We good to merge this?
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] datafusion/common: Add support for hashing ListView arrays [datafusion]
Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb commented on PR #19814:
URL: https://github.com/apache/datafusion/pull/19814#issuecomment-3759951646
I took a look at this PR and it looks good to me too -- thanks @brancz @martin-g and @Jefffrey
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org