Skip to content

Commit 46b508e

Browse files
authored
perf: optimize object store requests when reading CSV (apache#22962)
## Which issue does this PR close? - Closes apache#21419 ## Rationale for this change The CSV scanner currently uses `calculate_range` which issues two extra `get_opts` requests per byte range to find newline boundaries (one for the start boundary, one for the end boundary), plus one GET for the actual data. For a file split into 3 partitions, this results in 8 total object store requests. apache#20823 solved this same problem for the JSON scanner by introducing `AlignedBoundaryStream`, which wraps the raw byte stream and lazily aligns to newline boundaries as data is read, eliminating the extra boundary-seeking requests entirely. This PR applies the same approach to CSV. ## What changes are included in this PR? Based on the approach from apache#20823: Moved `AlignedBoundaryStream` from `datasource-json` to the shared `datasource` crate so it can be reused by both JSON and CSV scanners. Updated `CsvOpener` to use instead of `calculate_range`, and removed the `calculate_range` & `find_first_newline` as they no longer had any callers. Updated tests to reflect. Public API changes include: - Removal of the `RangeCalculation` enum - Removal of the `calculate_change` function - `boundary_stream` (containing `AlignedBoundaryStream`) moved from `datafusion_datasource_json` to `datafusion_datasource` ## Are these changes tested? Yes. The existing `AlignedBoundaryStream` unit tests (16 tests covering boundary alignment edge cases) were moved along with the implementation and continue to pass. The `query_csv_file_with_byte_range_partitions` snapshot test in `object_store_access.rs` has been updated to verify the new request pattern (4 requests instead of 8). ## Are there any user-facing changes? No.
1 parent ad1a260 commit 46b508e

7 files changed

Lines changed: 90 additions & 209 deletions

File tree

datafusion/core/tests/datasource/object_store_access.rs

Lines changed: 7 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -231,23 +231,13 @@ async fn query_multi_csv_file() {
231231
);
232232
}
233233

234-
/// Test that a CSV file split into byte ranges via repartitioning exercises
235-
/// range-based object store access.
234+
/// Test that a CSV file split into byte ranges via repartitioning produces
235+
/// exactly one GET request per byte range — no extra requests for boundary seeking.
236236
///
237237
/// With a single file and `target_partitions=3`, the repartitioner produces
238-
/// exactly 3 ranges. For each range, `calculate_range` calls
239-
/// `find_first_newline` via a GET for every non-file boundary it touches
240-
/// (the start boundary if `start > 0`, the end boundary if `end < file_size`),
241-
/// plus one GET for the actual data — so 2 GETs for the first range (end scan
242-
/// + data), 3 for the middle range (start scan + end scan + data), and 2 for
243-
/// the last range (start scan + data) = 7 data GETs total. Additionally,
244-
/// adjacent ranges share a boundary position, so each shared boundary is scanned
245-
/// twice — once as the left range's end and again as the right range's start —
246-
/// producing the duplicate GETs visible in the snapshot. Add the 1 HEAD for
247-
/// file-size metadata = **8 total**.
248-
///
249-
/// This differs from the JSON reader which uses [`AlignedBoundaryStream`] and
250-
/// needs only 1 GET per range.
238+
/// exactly 3 ranges. Each range is served by a single [`AlignedBoundaryStream`]
239+
/// which issues exactly one bounded `get_opts` call, so there are 3 data GETs
240+
/// plus 1 HEAD (to determine file size) = **4 total**.
251241
///
252242
/// This test documents the current request pattern to catch regressions.
253243
#[tokio::test]
@@ -275,15 +265,11 @@ async fn query_csv_file_with_byte_range_partitions() {
275265
+---------+-------+-------+
276266
------- Object Store Request Summary -------
277267
RequestCountingObjectStore()
278-
Total Requests: 8
268+
Total Requests: 4
279269
- GET (opts) path=csv_range_table.csv head=true
270+
- GET (opts) path=csv_range_table.csv range=0-129
280271
- GET (opts) path=csv_range_table.csv range=42-129
281-
- GET (opts) path=csv_range_table.csv range=0-49
282-
- GET (opts) path=csv_range_table.csv range=42-129
283-
- GET (opts) path=csv_range_table.csv range=85-129
284-
- GET (opts) path=csv_range_table.csv range=49-89
285272
- GET (opts) path=csv_range_table.csv range=85-129
286-
- GET (opts) path=csv_range_table.csv range=89-129
287273
"
288274
);
289275
}

datafusion/datasource-csv/src/source.rs

Lines changed: 47 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -17,24 +17,23 @@
1717

1818
//! Execution plan for reading CSV files
1919
20+
use datafusion_datasource::boundary_stream::AlignedBoundaryStream;
2021
use datafusion_datasource::projection::{ProjectionOpener, SplitProjection};
2122
use datafusion_physical_plan::projection::ProjectionExprs;
2223
use std::fmt;
23-
use std::io::{Read, Seek, SeekFrom};
24+
use std::io::Read;
2425
use std::sync::Arc;
25-
use std::task::Poll;
2626

2727
use datafusion_datasource::decoder::{DecoderDeserializer, deserialize_stream};
2828
use datafusion_datasource::file_compression_type::FileCompressionType;
2929
use datafusion_datasource::file_stream::{FileOpenFuture, FileOpener};
3030
use datafusion_datasource::{
31-
FileRange, ListingTableUrl, PartitionedFile, RangeCalculation, TableSchema,
32-
as_file_source, calculate_range,
31+
FileRange, ListingTableUrl, PartitionedFile, TableSchema, as_file_source,
3332
};
3433

3534
use arrow::csv;
3635
use datafusion_common::config::CsvOptions;
37-
use datafusion_common::{DataFusionError, Result};
36+
use datafusion_common::{DataFusionError, Result, exec_datafusion_err};
3837
use datafusion_common_runtime::JoinSet;
3938
use datafusion_datasource::file::FileSource;
4039
use datafusion_datasource::file_scan_config::FileScanConfig;
@@ -367,43 +366,53 @@ impl FileOpener for CsvOpener {
367366

368367
Ok(Box::pin(async move {
369368
// Current partition contains bytes [start_byte, end_byte) (might contain incomplete lines at boundaries)
369+
let file_size = partitioned_file.object_meta.size;
370+
let location = partitioned_file.object_meta.location;
371+
372+
if let Some(file_range) = partitioned_file.range.as_ref() {
373+
let raw_start: u64 = file_range.start.try_into().map_err(|_| {
374+
exec_datafusion_err!(
375+
"Expected start range to fit in u64, got {}",
376+
file_range.start
377+
)
378+
})?;
379+
let raw_end: u64 = file_range.end.try_into().map_err(|_| {
380+
exec_datafusion_err!(
381+
"Expected end range to fit in u64, got {}",
382+
file_range.end
383+
)
384+
})?;
385+
386+
let aligned_stream = AlignedBoundaryStream::new(
387+
Arc::clone(&store),
388+
location.clone(),
389+
raw_start,
390+
raw_end,
391+
file_size,
392+
terminator.unwrap_or(b'\n'),
393+
)
394+
.await?
395+
.map_err(DataFusionError::from);
396+
397+
let decoder = config.builder().build_decoder();
398+
let input = file_compression_type
399+
.convert_stream(aligned_stream.boxed())?
400+
.fuse();
401+
let stream = deserialize_stream(
402+
input,
403+
DecoderDeserializer::new(CsvDecoder::new(decoder)),
404+
);
405+
return Ok(stream.map_err(Into::into).boxed());
406+
}
370407

371-
let calculated_range =
372-
calculate_range(&partitioned_file, &store, terminator).await?;
373-
374-
let range = match calculated_range {
375-
RangeCalculation::Range(None) => None,
376-
RangeCalculation::Range(Some(range)) => Some(range.into()),
377-
RangeCalculation::TerminateEarly => {
378-
return Ok(
379-
futures::stream::poll_fn(move |_| Poll::Ready(None)).boxed()
380-
);
381-
}
382-
};
383-
384-
let options = GetOptions {
385-
range,
386-
..Default::default()
387-
};
388-
389-
let result = store
390-
.get_opts(&partitioned_file.object_meta.location, options)
391-
.await?;
408+
// No range specified — read the entire file
409+
let options = GetOptions::default();
410+
let result = store.get_opts(&location, options).await?;
392411

393412
match result.payload {
394413
#[cfg(not(target_arch = "wasm32"))]
395-
GetResultPayload::File(mut file, _) => {
396-
let is_whole_file_scanned = partitioned_file.range.is_none();
397-
let decoder = if is_whole_file_scanned {
398-
// Don't seek if no range as breaks FIFO files
399-
file_compression_type.convert_read(file)?
400-
} else {
401-
file.seek(SeekFrom::Start(result.range.start as _))?;
402-
file_compression_type.convert_read(
403-
file.take((result.range.end - result.range.start) as u64),
404-
)?
405-
};
406-
414+
GetResultPayload::File(file, _) => {
415+
let decoder = file_compression_type.convert_read(file)?;
407416
let mut reader = config.open(decoder)?;
408417

409418
// Use std::iter::from_fn to wrap execution of iterator's next() method.

datafusion/datasource-json/src/mod.rs

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,6 @@
2020
// https://github.com/apache/datafusion/issues/11143
2121
#![cfg_attr(not(test), deny(clippy::clone_on_ref_ptr))]
2222

23-
pub mod boundary_stream;
2423
pub mod file_format;
2524
pub mod source;
2625
pub mod utils;

datafusion/datasource-json/src/source.rs

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -25,11 +25,10 @@ use std::task::{Context, Poll};
2525
use crate::file_format::JsonDecoder;
2626
use crate::utils::{ChannelReader, JsonArrayToNdjsonReader};
2727

28-
use crate::boundary_stream::AlignedBoundaryStream;
29-
3028
use datafusion_common::error::{DataFusionError, Result};
3129
use datafusion_common::exec_datafusion_err;
3230
use datafusion_common_runtime::{JoinSet, SpawnedTask};
31+
use datafusion_datasource::boundary_stream::AlignedBoundaryStream;
3332
use datafusion_datasource::decoder::{DecoderDeserializer, deserialize_stream};
3433
use datafusion_datasource::file_compression_type::FileCompressionType;
3534
use datafusion_datasource::file_stream::{FileOpenFuture, FileOpener};

datafusion/datasource-json/src/boundary_stream.rs renamed to datafusion/datasource/src/boundary_stream.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@
1515
// specific language governing permissions and limitations
1616
// under the License.
1717

18-
//! Streaming boundary-aligned wrapper for newline-delimited JSON range reads.
18+
//! Streaming boundary-aligned wrapper for newline-delimited JSON and CSV range reads.
1919
//!
2020
//! [`AlignedBoundaryStream`] wraps a raw byte stream and lazily aligns to
2121
//! record (newline) boundaries, avoiding the need for separate `get_opts`
@@ -398,7 +398,7 @@ impl Stream for AlignedBoundaryStream {
398398
#[cfg(test)]
399399
mod tests {
400400
use super::*;
401-
use crate::test_utils::{CHUNK_SIZES, make_chunked_store};
401+
use crate::test_util::{CHUNK_SIZES, make_chunked_store};
402402
use futures::TryStreamExt;
403403

404404
async fn collect_stream(stream: AlignedBoundaryStream) -> Vec<u8> {

datafusion/datasource/src/mod.rs

Lines changed: 4 additions & 145 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@
2828
//! A table that uses the `ObjectStore` listing capability
2929
//! to get the list of files to process.
3030
31+
pub mod boundary_stream;
3132
pub mod decoder;
3233
pub mod display;
3334
pub mod file;
@@ -56,11 +57,10 @@ pub use self::url::ListingTableUrl;
5657
use crate::file_groups::FileGroup;
5758
use chrono::TimeZone;
5859
use datafusion_common::stats::Precision;
59-
use datafusion_common::{ColumnStatistics, Result, TableReference, exec_datafusion_err};
60+
use datafusion_common::{ColumnStatistics, Result, TableReference};
6061
use datafusion_common::{ScalarValue, Statistics};
6162
use datafusion_physical_expr::LexOrdering;
62-
use futures::{Stream, StreamExt};
63-
use object_store::{GetOptions, GetRange, ObjectStore};
63+
use futures::Stream;
6464
use object_store::{ObjectMeta, path::Path};
6565
pub use table_schema::{TableSchema, TableSchemaBuilder};
6666
// Remove when add_row_stats is remove
@@ -69,7 +69,6 @@ use arrow::datatypes::SchemaRef;
6969
pub use statistics::add_row_stats;
7070
pub use statistics::compute_all_files_statistics;
7171
use std::any::Any;
72-
use std::ops::Range;
7372
use std::pin::Pin;
7473
use std::sync::Arc;
7574

@@ -411,119 +410,6 @@ impl From<ObjectMeta> for PartitionedFile {
411410
}
412411
}
413412

414-
/// Represents the possible outcomes of a range calculation.
415-
///
416-
/// This enum is used to encapsulate the result of calculating the range of
417-
/// bytes to read from an object (like a file) in an object store.
418-
///
419-
/// Variants:
420-
/// - `Range(Option<Range<usize>>)`:
421-
/// Represents a range of bytes to be read. It contains an `Option` wrapping a
422-
/// `Range<usize>`. `None` signifies that the entire object should be read,
423-
/// while `Some(range)` specifies the exact byte range to read.
424-
/// - `TerminateEarly`:
425-
/// Indicates that the range calculation determined no further action is
426-
/// necessary, possibly because the calculated range is empty or invalid.
427-
pub enum RangeCalculation {
428-
Range(Option<Range<u64>>),
429-
TerminateEarly,
430-
}
431-
432-
/// Calculates an appropriate byte range for reading from an object based on the
433-
/// provided metadata.
434-
///
435-
/// This asynchronous function examines the [`PartitionedFile`] of an object in an object store
436-
/// and determines the range of bytes to be read. The range calculation may adjust
437-
/// the start and end points to align with meaningful data boundaries (like newlines).
438-
///
439-
/// Returns a `Result` wrapping a [`RangeCalculation`], which is either a calculated byte range or an indication to terminate early.
440-
///
441-
/// Returns an `Error` if any part of the range calculation fails, such as issues in reading from the object store or invalid range boundaries.
442-
pub async fn calculate_range(
443-
file: &PartitionedFile,
444-
store: &Arc<dyn ObjectStore>,
445-
terminator: Option<u8>,
446-
) -> Result<RangeCalculation> {
447-
let location = &file.object_meta.location;
448-
let file_size = file.object_meta.size;
449-
let newline = terminator.unwrap_or(b'\n');
450-
451-
match file.range {
452-
None => Ok(RangeCalculation::Range(None)),
453-
Some(FileRange { start, end }) => {
454-
let start: u64 = start.try_into().map_err(|_| {
455-
exec_datafusion_err!("Expect start range to fit in u64, got {start}")
456-
})?;
457-
let end: u64 = end.try_into().map_err(|_| {
458-
exec_datafusion_err!("Expect end range to fit in u64, got {end}")
459-
})?;
460-
461-
let start_delta = if start != 0 {
462-
find_first_newline(store, location, start - 1, file_size, newline).await?
463-
} else {
464-
0
465-
};
466-
467-
if start + start_delta > end {
468-
return Ok(RangeCalculation::TerminateEarly);
469-
}
470-
471-
let end_delta = if end != file_size {
472-
find_first_newline(store, location, end - 1, file_size, newline).await?
473-
} else {
474-
0
475-
};
476-
477-
let range = start + start_delta..end + end_delta;
478-
479-
if range.start >= range.end {
480-
return Ok(RangeCalculation::TerminateEarly);
481-
}
482-
483-
Ok(RangeCalculation::Range(Some(range)))
484-
}
485-
}
486-
}
487-
488-
/// Asynchronously finds the position of the first newline character in a specified byte range
489-
/// within an object, such as a file, in an object store.
490-
///
491-
/// This function scans the contents of the object starting from the specified `start` position
492-
/// up to the `end` position, looking for the first occurrence of a newline character.
493-
/// It returns the position of the first newline relative to the start of the range.
494-
///
495-
/// Returns a `Result` wrapping a `usize` that represents the position of the first newline character found within the specified range. If no newline is found, it returns the length of the scanned data, effectively indicating the end of the range.
496-
///
497-
/// The function returns an `Error` if any issues arise while reading from the object store or processing the data stream.
498-
async fn find_first_newline(
499-
object_store: &Arc<dyn ObjectStore>,
500-
location: &Path,
501-
start: u64,
502-
end: u64,
503-
newline: u8,
504-
) -> Result<u64> {
505-
let options = GetOptions {
506-
range: Some(GetRange::Bounded(start..end)),
507-
..Default::default()
508-
};
509-
510-
let result = object_store.get_opts(location, options).await?;
511-
let mut result_stream = result.into_stream();
512-
513-
let mut index = 0;
514-
515-
while let Some(chunk) = result_stream.next().await.transpose()? {
516-
if let Some(position) = chunk.iter().position(|&byte| byte == newline) {
517-
let position = position as u64;
518-
return Ok(index + position);
519-
}
520-
521-
index += chunk.len() as u64;
522-
}
523-
524-
Ok(index)
525-
}
526-
527413
/// Generates test files with min-max statistics in different overlap patterns.
528414
///
529415
/// Used by tests and benchmarks.
@@ -647,7 +533,7 @@ mod tests {
647533
use datafusion_execution::object_store::{
648534
DefaultObjectStoreRegistry, ObjectStoreRegistry,
649535
};
650-
use object_store::{ObjectStoreExt, local::LocalFileSystem, path::Path};
536+
use object_store::{local::LocalFileSystem, path::Path};
651537
use std::{collections::HashMap, ops::Not, sync::Arc};
652538
use url::Url;
653539

@@ -844,31 +730,4 @@ mod tests {
844730
// testing an empty path with `ignore_subdirectory` set to false
845731
assert!(url.contains(&Path::parse("/var/data/mytable/").unwrap(), false));
846732
}
847-
848-
/// Regression test for <https://github.com/apache/datafusion/issues/19605>
849-
#[tokio::test]
850-
async fn test_calculate_range_single_line_file() {
851-
use super::{PartitionedFile, RangeCalculation, calculate_range};
852-
use object_store::ObjectStore;
853-
use object_store::memory::InMemory;
854-
855-
let content = r#"{"id":1,"data":"aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa"}"#;
856-
let file_size = content.len() as u64;
857-
858-
let store: Arc<dyn ObjectStore> = Arc::new(InMemory::new());
859-
let path = Path::from("test.json");
860-
store.put(&path, content.into()).await.unwrap();
861-
862-
let mid = file_size / 2;
863-
let partitioned_file = PartitionedFile::new_with_range(
864-
path.to_string(),
865-
file_size,
866-
mid as i64,
867-
file_size as i64,
868-
);
869-
870-
let result = calculate_range(&partitioned_file, &store, None).await;
871-
872-
assert!(matches!(result, Ok(RangeCalculation::TerminateEarly)));
873-
}
874733
}

0 commit comments

Comments
 (0)