Skip to content

Commit 8f0f23a

Browse files
committed
fixup! fix: support total_size=0 in byte_range_to_row_range
Address review feedback by adding an early exit check to VortexOpener::open in order to avoid even more work: ```rust if vxf.row_count() == 0 { let empty_stream = stream::iter(vec![]).boxed(); return Ok(empty_stream); } ``` The `byte_range_to_row_range` is now protected with a `debug_assert!` call that asserts the expected `row_count > 0` invariant.
1 parent 3388c15 commit 8f0f23a

1 file changed

Lines changed: 35 additions & 8 deletions

File tree

vortex-datafusion/src/persistent/opener.rs

Lines changed: 35 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -198,6 +198,13 @@ impl FileOpener for VortexOpener {
198198
.await
199199
.map_err(|e| exec_datafusion_err!("Failed to open Vortex file {e}"))?;
200200

201+
if vxf.row_count() == 0 {
202+
// There are no rows in this file: we can save ourselves some
203+
// work and return an empty stream immediately.
204+
let empty_stream = stream::iter(vec![]).boxed();
205+
return Ok(empty_stream);
206+
}
207+
201208
// This is the expected arrow types of the actual columns in the file, which might have different types
202209
// from the unified logical schema or miss
203210
let this_file_schema = Arc::new(calculate_physical_schema(
@@ -430,11 +437,7 @@ fn apply_byte_range(
430437
}
431438

432439
fn byte_range_to_row_range(byte_range: Range<u64>, row_count: u64, total_size: u64) -> Range<u64> {
433-
// Datafusion might generate ranges for files with no rows. In such cases,
434-
// we return an empty range.
435-
if row_count == 0 {
436-
return 0..0;
437-
}
440+
debug_assert!(row_count > 0); // Asserted by an early exit check in VortexOpener::open
438441

439442
let average_row = total_size / row_count;
440443
assert!(average_row > 0, "A row must always have at least one byte");
@@ -501,9 +504,6 @@ mod tests {
501504
#[case(50..105, 100, 105, 50..100)]
502505
#[case(0..1, 4, 8, 0..0)]
503506
#[case(1..8, 4, 8, 0..4)]
504-
#[case(0..100, 0, 100, 0..0)]
505-
#[case(10..50, 0, 0, 0..0)]
506-
#[case(0..1, 0, 1, 0..0)]
507507
fn test_range_translation(
508508
#[case] byte_range: Range<u64>,
509509
#[case] row_count: u64,
@@ -628,6 +628,33 @@ mod tests {
628628
Ok(())
629629
}
630630

631+
#[tokio::test]
632+
async fn test_open_empty_file() -> anyhow::Result<()> {
633+
use futures::TryStreamExt;
634+
635+
let object_store = Arc::new(InMemory::new()) as Arc<dyn ObjectStore>;
636+
let data_batch = record_batch!(("a", Int32, Vec::<i32>::new())).unwrap();
637+
let file_path = "part=1/empty.vortex";
638+
let file_size =
639+
write_arrow_to_vortex(object_store.clone(), file_path, data_batch.clone()).await?;
640+
641+
let file_schema = data_batch.schema();
642+
// Parallel scans may attach a byte range even for empty files; the
643+
// opener must not call byte_range_to_row_range when the row_count is 0.
644+
let file =
645+
PartitionedFile::new_with_range(file_path.to_string(), file_size, 0, file_size as i64);
646+
647+
let table_schema = TableSchema::from_file_schema(file_schema.clone());
648+
649+
let opener = make_opener(object_store, table_schema, None);
650+
let stream = opener.open(file)?.await?;
651+
let data = stream.try_collect::<Vec<_>>().await?;
652+
653+
assert_eq!(data.len(), 0);
654+
655+
Ok(())
656+
}
657+
631658
#[rstest]
632659
#[tokio::test]
633660
async fn test_open_files_different_table_schema() -> anyhow::Result<()> {

0 commit comments

Comments
 (0)