sunchao commented on code in PR #7173:
URL: https://github.com/apache/datafusion-comet/pull/7173#discussion_r4365206512
##########
native/spark-expr/src/bloom_filter/bloom_filter_agg.rs:
##########
@@ -190,16 +190,83 @@ impl Accumulator for SparkBloomFilter {
"Expect one element in 'states' but found {}",
states.len()
);
- assert_eq!(states[0].len(), 1);
- let state_sv = downcast_value!(states[0], BinaryArray);
- self.merge_filter(state_sv.value_data())
+ let states = downcast_value!(states[0], BinaryArray);
+ // Spill replay can supply multiple states for a group. Use each row's
+ // slice so that sliced arrays also honor their offsets.
Review Comment:
The comment in 9a83585f6 and the PR rationale now describe ordinary final
aggregation. A group’s Arrow slice can share its backing buffer with other
groups, which is why `merge_batch` uses each row’s logical bytes. I also
checked this by restoring `value_data()` only for one-row groups. The two-state
group still merged correctly, and the new regression caught the wrong filter
bytes for the other group.
--
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]
