Skip to content

Commit 7ba790f

Browse files
authored
Preserve selection in the presence of a row range (vortex-data#9382)
Prune splits for a Selection even when a row range is passed Signed-off-by: Nicholas Gates <nick@nickgates.com>
1 parent 4273f37 commit 7ba790f

1 file changed

Lines changed: 58 additions & 6 deletions

File tree

vortex-layout/src/scan/splits.rs

Lines changed: 58 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -38,13 +38,19 @@ pub fn attempt_split_ranges(
3838
return None;
3939
};
4040

41-
// TODO(connor): We can be smarter here, as the row range is more restrictive than the
42-
// selection.
43-
if row_range.is_some() {
44-
return None;
45-
}
46-
4741
let indices = buffer.as_slice();
42+
let indices = if let Some(row_range) = row_range {
43+
if row_range.is_empty() {
44+
return Some(Vec::new());
45+
}
46+
47+
let start = indices.partition_point(|&index| index < row_range.start);
48+
let end = indices.partition_point(|&index| index < row_range.end);
49+
&indices[start..end]
50+
} else {
51+
indices
52+
};
53+
4854
if indices.is_empty() {
4955
return Some(Vec::new());
5056
}
@@ -86,3 +92,49 @@ pub fn attempt_split_ranges(
8692

8793
Some(ranges)
8894
}
95+
96+
#[cfg(test)]
97+
mod tests {
98+
use vortex_buffer::Buffer;
99+
use vortex_error::VortexExpect;
100+
use vortex_scan::strict_sorted_buffer::StrictSortedBuffer;
101+
102+
use super::*;
103+
104+
fn include(indices: impl IntoIterator<Item = u64>) -> Selection {
105+
Selection::IncludeByIndex(
106+
StrictSortedBuffer::try_new(Buffer::from_iter(indices))
107+
.vortex_expect("test indices must be strictly sorted"),
108+
)
109+
}
110+
111+
#[test]
112+
fn split_ranges_intersect_row_range() {
113+
let ranges = attempt_split_ranges(&include([1, 3, 5, 7, 9]), Some(&(3..9)))
114+
.vortex_expect("sparse split planning should apply");
115+
116+
assert_eq!(ranges.len(), 1);
117+
assert_eq!(ranges[0], 3..8);
118+
}
119+
120+
#[test]
121+
fn split_ranges_empty_intersection() {
122+
assert_eq!(
123+
attempt_split_ranges(&include([1, 3, 5]), Some(&(6..9))),
124+
Some(Vec::new())
125+
);
126+
}
127+
128+
#[test]
129+
fn restrictive_row_range_enables_sparse_splits() {
130+
let indices = (0..MAX_RANGE_SIZE).chain([MAX_RANGE_SIZE + MIN_GAP_BETWEEN_RANGES]);
131+
let ranges = attempt_split_ranges(&include(indices), Some(&(MAX_RANGE_SIZE..u64::MAX)))
132+
.vortex_expect("the row range should make sparse split planning applicable");
133+
134+
assert_eq!(ranges.len(), 1);
135+
assert_eq!(
136+
ranges[0],
137+
MAX_RANGE_SIZE + MIN_GAP_BETWEEN_RANGES..MAX_RANGE_SIZE + MIN_GAP_BETWEEN_RANGES + 1
138+
);
139+
}
140+
}

0 commit comments

Comments
 (0)