Vcf byte range#45
Open
amd-pdebski wants to merge 15 commits into
Open
Conversation
…boundaries - Fix LimitedRangeFile to properly enforce byte range limits during reading - Add proper FASTQ record boundary detection for splits - Implement LimitedRangeFile wrapper that stops reading at end offset - Add FastqLocalReader::PlainRanged variant for byte range reading - Add comprehensive unit tests for byte range functionality - Fix missing benchmark_bgzf_threads.rs reference in bio-format-gff This resolves the issue where FASTQ readers were reading entire files instead of respecting byte ranges, causing poor parallelization performance. Tests verify: - Proper byte range enforcement - FASTQ record boundary handling - Various split scenarios (start, middle, end) - Integration with FastqLocalReader - Error handling for invalid ranges
The previous implementation only checked if a line starts with '@', which fails because FASTQ quality scores can contain '@' characters (ASCII 64 = Q31). This caused false positive matches and incorrect partition boundaries. Changes: - Implement sliding window approach that checks 4-line patterns - Validate complete FASTQ records: @Header, sequence, +separator, quality - Add strong validation: sequence length must equal quality length - Search up to 2000 bytes beyond partition end to find valid boundary - Skip initial partial line before starting pattern search This ensures all partitions find correct record boundaries, fixing the issue where some partitions would return 0 records due to false '@' matches in quality scores. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
This commit implements byte range support for VCF files, enabling
distributed/partitioned reading of VCF files similar to the FASTQ
implementation. The implementation follows the same architectural
pattern while handling VCF-specific requirements, particularly header
reading.
Key Changes:
1. Type and Provider Layer:
- Added VcfByteRange struct for byte range specification
- Added byte_range field to VcfTableProvider
- Added new_with_range() method that always reads header from byte 0
for schema inference, regardless of byte_range.start
- Added new_async() and new_with_range_async() methods to prevent
deadlocks when called from async contexts (fixes block_on deadlock)
- Updated scan() to pass byte_range to VcfExec
2. Execution Layer:
- Added byte_range field to VcfExec
- Updated execute() and stream functions to handle byte ranges
- Implemented header reading strategy: read from byte 0 when
byte_range.start == 0, otherwise use pre-read header from schema
inference
3. Storage Functions:
- get_local_vcf_bgzf_reader_with_range(): BGZF range reading with
GZI index support
- get_local_vcf_reader_with_range(): Uncompressed range reading
- open_local_vcf_reader_at_range(): Helper for finding variant line
boundaries (skips header lines starting with '#')
- get_remote_vcf_bgzf_reader_with_range(): Remote BGZF range reading
- get_remote_vcf_reader_with_range(): Remote uncompressed range reading
4. Reader Enums:
- Added BGZFRanged variant to VcfLocalReader for BGZF range reading
- Added PlainRanged variant to VcfLocalReader for uncompressed range
- Added new_with_range() to both VcfLocalReader and VcfRemoteReader
- Updated read_records(), read_header(), and describe() methods to
handle new variants
5. Testing:
- Added byte_range_test.rs with 6 comprehensive tests
- Added range_row_count_regression.rs with DataFusion integration
tests
- All tests passing
VCF-Specific Considerations:
- Header Reading: VCF headers must be read from byte 0 for schema
inference, regardless of byte_range.start. The byte_range only applies
to variant record reading, ensuring schema consistency across all
splits.
- Schema Consistency: Schema is determined once during
VcfTableProvider::new_with_range() by reading header from byte 0.
All splits use the same schema stored in the provider.
- Line-Based Boundaries: VCF is line-based, making record boundary
finding simpler than FASTQ's 4-line records. The implementation
skips header lines (starting with '#') and reads complete variant
lines.
- GZIP Error Handling: Regular GZIP files return an error if
byte_range.start > 0, as they cannot be split (must use BGZF format
for distributed reading).
- Async Safety: Added async versions (new_async, new_with_range_async)
to prevent deadlocks when called from async contexts. The sync versions
use block_on internally and should only be called from non-async code.
The implementation follows the FASTQ pattern exactly, with the critical
difference being header handling: header from byte 0 for schema,
byte_range for variants only.
All tests pass and the code compiles without errors.
When noodles-vcf hits EOF unexpectedly (e.g., missing trailing newline or reading past end of file), it returns an error instead of None. This causes 'unexpected EOL' errors that fail tests. This fix: - Detects EOF-related errors by checking error message for 'eol', 'eof', 'unexpected end' keywords - Checks for IO errors with UnexpectedEof kind - Breaks gracefully on EOF errors instead of propagating them - Still propagates other legitimate errors This matches the pattern used in FASTQ and BED implementations where EOF is handled as a normal end-of-stream condition rather than an error. Fixes hanging tests in comet-bio that were failing with 'unexpected EOL' errors when reading VCF files.
After calling read_header() on a noodles-vcf reader, the reader may be in a state where read_records() returns an empty stream. This causes the stream to return None immediately, producing no batches. Fix: Always read the header separately (from a different reader instance) before creating the reader for record reading. This matches the pattern used in bgzf_parallel_reader.rs where the header is read from a separate file/reader instance. This ensures the reader used for record reading is in a clean state and can properly read records after the header. Fixes issue where stream returns None immediately after header reading.
noodles-vcf requires the header to be read on the same reader instance before calling records(). Even though we already read the header separately for schema purposes, we must also read it on the reader used for record reading to position it correctly. This matches the pattern in debug_vcf_reading.rs where: 1. Create reader 2. Call read_header() on the reader 3. Then call records() on the same reader The fix: - Read header separately for schema (unchanged) - Create new reader for record reading - Read header on the new reader (discard result, we already have it) - Then call read_records() - now works correctly Fixes issue where stream returns None immediately because reader wasn't positioned correctly after header reading.
Implements byte range reading for VCF files, enabling parallel and distributed processing similar to FASTQ implementation. Key features: - Byte range support for BGZF, GZIP, and uncompressed VCF files - Header always read from byte 0 for schema consistency across splits - Proper handling of record boundaries and partial records - GZI index support for BGZF files - Async provider creation to avoid deadlocks Implementation details: - Added VcfByteRange struct and new_with_range methods - Implemented ranged readers for all compression types - Added header reading on record reader for noodles-vcf compatibility - Graceful EOF error handling for noodles-vcf behavior - Comprehensive test coverage (6/6 tests passing) This enables partitioned VCF file reading in distributed systems while maintaining schema consistency across all partitions.
amd-pdebski
force-pushed
the
vcf-byte-range
branch
5 times, most recently
from
January 3, 2026 15:37
bead442 to
89796bf
Compare
Resolved all conflicts by: - Keeping VCF byte range functionality (VcfByteRange, new_with_range methods, BGZFRanged/PlainRanged variants) - Keeping FASTQ byte range functionality - Merging master's documentation improvements - Using master's DataFusion 50.3.0 and updated dependencies - Regenerating Cargo.lock from master All conflicts resolved, code compiles, tests passing. Additional changes: - Fix missing documentation warnings in FASTQ and VCF modules - Add VCF benchmark configuration (benchmarks/configs/vcf.yml) - Update CI/CD workflow to run VCF benchmarks alongside GFF - Fix benchmark runner speedup display formatting
amd-pdebski
force-pushed
the
vcf-byte-range
branch
6 times, most recently
from
January 4, 2026 11:04
b8a589a to
553ead7
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.