Skip to content

Commit ec32ac3

Browse files
authored
Support nested REE in arrow-ord partition function (#9642)
# Which issue does this PR close? <!-- We generally require a GitHub issue to be filed for all bug fixes and enhancements and this helps us generate change logs for our releases. You can link an issue to this PR using the GitHub syntax. --> - Closes #9640. # Rationale for this change <!-- Why are you proposing this change? If this is already explained clearly in the issue then this section is not needed. Explaining clearly why changes are proposed helps reviewers understand your changes and offer better suggestions for fixes. --> Although rare, it's totally valid to have nested REE and dict encoding and we should handle it correctly. # What changes are included in this PR? <!-- There is no need to duplicate the description in the issue here but it is sometimes worth providing a summary of the individual changes in this PR. --> Process nested REE, nested dict, dict of REE, REE of dict gracefully # Are these changes tested? <!-- We typically require tests for all PRs in order to: 1. Prevent the code from being accidentally broken by subsequent changes 2. Serve as another way to document the expected behavior of the code If tests are not included in your PR, please explain why (for example, are they covered by existing tests)? --> Yes # Are there any user-facing changes? <!-- If there are user-facing changes then we may require documentation to be updated before approving the PR. If there are any breaking changes to public APIs, please call them out. -->
1 parent ade0381 commit ec32ac3

File tree

2 files changed

+131
-3
lines changed

2 files changed

+131
-3
lines changed

arrow-ord/src/cmp.rs

Lines changed: 47 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,7 @@ use arrow_array::{
3131
};
3232
use arrow_buffer::bit_util::ceil;
3333
use arrow_buffer::{BooleanBuffer, NullBuffer};
34-
use arrow_schema::ArrowError;
34+
use arrow_schema::{ArrowError, DataType};
3535
use arrow_select::take::take;
3636
use std::cmp::Ordering;
3737
use std::ops::Not;
@@ -201,6 +201,20 @@ pub fn not_distinct(lhs: &dyn Datum, rhs: &dyn Datum) -> Result<BooleanArray, Ar
201201
compare_op(Op::NotDistinct, lhs, rhs)
202202
}
203203

204+
/// Returns true if `distinct` (via `compare_op`) can handle this data type.
205+
///
206+
/// `compare_op` unwraps at most one level of dictionary, then dispatches on
207+
/// the leaf type. Anything else (REE, nested dictionary, nested/complex types)
208+
/// must go through `make_comparator` instead.
209+
pub(crate) fn supports_distinct(dt: &DataType) -> bool {
210+
use arrow_schema::DataType::*;
211+
let leaf = match dt {
212+
Dictionary(_, v) => v.as_ref(),
213+
dt => dt,
214+
};
215+
!leaf.is_nested() && !matches!(leaf, Dictionary(_, _) | RunEndEncoded(_, _))
216+
}
217+
204218
/// Perform `op` on the provided `Datum`
205219
#[inline(never)]
206220
fn compare_op(op: Op, lhs: &dyn Datum, rhs: &dyn Datum) -> Result<BooleanArray, ArrowError> {
@@ -832,6 +846,38 @@ mod tests {
832846
assert_eq!(not_distinct(&b, &a).unwrap(), expected);
833847
}
834848

849+
#[test]
850+
fn test_supports_distinct() {
851+
use arrow_schema::{DataType::*, Field};
852+
853+
assert!(supports_distinct(&Int32));
854+
assert!(supports_distinct(&Float64));
855+
assert!(supports_distinct(&Utf8));
856+
assert!(supports_distinct(&Boolean));
857+
858+
// One level of dictionary unwrap is supported.
859+
assert!(supports_distinct(&Dictionary(
860+
Box::new(Int16),
861+
Box::new(Utf8),
862+
)));
863+
864+
// REE, nested dictionary, and complex types are not supported.
865+
assert!(!supports_distinct(&RunEndEncoded(
866+
Arc::new(Field::new("run_ends", Int32, false)),
867+
Arc::new(Field::new("values", Int32, true)),
868+
)));
869+
assert!(!supports_distinct(&Dictionary(
870+
Box::new(Int16),
871+
Box::new(Dictionary(Box::new(Int8), Box::new(Utf8))),
872+
)));
873+
assert!(!supports_distinct(&List(Arc::new(Field::new(
874+
"item", Int32, true,
875+
)))));
876+
assert!(!supports_distinct(&Struct(
877+
vec![Field::new("a", Int32, true)].into()
878+
)));
879+
}
880+
835881
#[test]
836882
fn test_scalar_negation() {
837883
let a = Int32Array::new_scalar(54);

arrow-ord/src/partition.rs

Lines changed: 84 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@ use arrow_array::{Array, ArrayRef};
2323
use arrow_buffer::BooleanBuffer;
2424
use arrow_schema::{ArrowError, SortOptions};
2525

26-
use crate::cmp::distinct;
26+
use crate::cmp::{distinct, supports_distinct};
2727
use crate::ord::make_comparator;
2828

2929
/// A computed set of partitions, see [`partition`]
@@ -158,7 +158,7 @@ fn find_boundaries(v: &dyn Array) -> Result<BooleanBuffer, ArrowError> {
158158
let v1 = v.slice(0, slice_len);
159159
let v2 = v.slice(1, slice_len);
160160

161-
if !v.data_type().is_nested() {
161+
if supports_distinct(v.data_type()) {
162162
return Ok(distinct(&v1, &v2)?.values().clone());
163163
}
164164
// Given that we're only comparing values, null ordering in the input or
@@ -306,6 +306,88 @@ mod tests {
306306
);
307307
}
308308

309+
#[test]
310+
fn test_partition_run_end_encoded() {
311+
let run_ends = Int32Array::from(vec![2, 3, 5]);
312+
let values = StringArray::from(vec!["x", "y", "x"]);
313+
let ree = RunArray::try_new(&run_ends, &values).unwrap();
314+
// logical: ["x", "x", "y", "x", "x"]
315+
let input = vec![Arc::new(ree) as _];
316+
assert_eq!(partition(&input).unwrap().ranges(), vec![0..2, 2..3, 3..5],);
317+
}
318+
319+
#[test]
320+
fn test_partition_nested_run_end_encoded() {
321+
// Inner REE (values of the outer): run_ends [1, 2, 3], values ["x", "y", "x"]
322+
// logical length 3: ["x", "y", "x"]
323+
let inner_run_ends = Int32Array::from(vec![1, 2, 3]);
324+
let inner_values = StringArray::from(vec!["x", "y", "x"]);
325+
let inner_ree = RunArray::try_new(&inner_run_ends, &inner_values).unwrap();
326+
327+
// Outer REE: run_ends [2, 3, 5], values = inner_ree (length 3)
328+
// logical: rows 0,1 → inner[0]="x", row 2 → inner[1]="y", rows 3,4 → inner[2]="x"
329+
// = ["x", "x", "y", "x", "x"]
330+
let outer_run_ends = Int32Array::from(vec![2, 3, 5]);
331+
let outer_ree = RunArray::try_new(&outer_run_ends, &inner_ree).unwrap();
332+
333+
let input = vec![Arc::new(outer_ree) as ArrayRef];
334+
assert_eq!(partition(&input).unwrap().ranges(), vec![0..2, 2..3, 3..5]);
335+
}
336+
337+
#[test]
338+
fn test_partition_ree_with_dictionary_values() {
339+
// Dictionary values: keys [0, 1, 0], dict ["x", "y"] → logical ["x", "y", "x"]
340+
let dict_values = StringArray::from(vec!["x", "y"]);
341+
let keys = Int32Array::from(vec![0, 1, 0]);
342+
let dict = DictionaryArray::try_new(keys, Arc::new(dict_values)).unwrap();
343+
344+
// REE wrapping dict: run_ends [2, 3, 5] → logical [dict[0], dict[0], dict[1], dict[2], dict[2]]
345+
// = ["x", "x", "y", "x", "x"]
346+
let run_ends = Int32Array::from(vec![2, 3, 5]);
347+
let ree = RunArray::try_new(&run_ends, &dict).unwrap();
348+
let input = vec![Arc::new(ree) as ArrayRef];
349+
assert_eq!(partition(&input).unwrap().ranges(), vec![0..2, 2..3, 3..5],);
350+
}
351+
352+
#[test]
353+
fn test_partition_dictionary() {
354+
let values = StringArray::from(vec!["x", "y"]);
355+
let keys = Int32Array::from(vec![0, 0, 1, 0, 0]);
356+
let dict = DictionaryArray::try_new(keys, Arc::new(values)).unwrap();
357+
// logical: ["x", "x", "y", "x", "x"]
358+
let input = vec![Arc::new(dict) as _];
359+
assert_eq!(partition(&input).unwrap().ranges(), vec![0..2, 2..3, 3..5],);
360+
}
361+
362+
#[test]
363+
fn test_partition_nested_dictionary() {
364+
let inner_values = StringArray::from(vec!["x", "y"]);
365+
let inner_keys = Int32Array::from(vec![0, 1, 0]);
366+
let inner_dict = DictionaryArray::try_new(inner_keys, Arc::new(inner_values)).unwrap();
367+
368+
// Outer dict keys index into inner dict's logical values: ["x", "y", "x"]
369+
// keys [0, 0, 1, 2, 2] → logical ["x", "x", "y", "x", "x"]
370+
let outer_keys = Int32Array::from(vec![0, 0, 1, 2, 2]);
371+
let outer_dict = DictionaryArray::try_new(outer_keys, Arc::new(inner_dict)).unwrap();
372+
let input = vec![Arc::new(outer_dict) as ArrayRef];
373+
assert_eq!(partition(&input).unwrap().ranges(), vec![0..2, 2..3, 3..5],);
374+
}
375+
376+
#[test]
377+
fn test_partition_dictionary_with_ree_values() {
378+
// REE values: run_ends [2, 3], values ["x", "y"] → logical ["x", "x", "y"]
379+
let run_ends = Int32Array::from(vec![2, 3]);
380+
let str_values = StringArray::from(vec!["x", "y"]);
381+
let ree = RunArray::try_new(&run_ends, &str_values).unwrap();
382+
383+
// Dictionary keys index into the REE's logical values
384+
// keys [0, 0, 2, 0, 0] → logical ["x", "x", "y", "x", "x"]
385+
let keys = Int32Array::from(vec![0, 0, 2, 0, 0]);
386+
let dict = DictionaryArray::try_new(keys, Arc::new(ree)).unwrap();
387+
let input = vec![Arc::new(dict) as ArrayRef];
388+
assert_eq!(partition(&input).unwrap().ranges(), vec![0..2, 2..3, 3..5],);
389+
}
390+
309391
#[test]
310392
fn test_partition_nested() {
311393
let input = vec![

0 commit comments

Comments
 (0)