Skip to content

fix(read): validate byte ranges and honor disabled prefetch cache - #522

Merged
lxy-9602 merged 2 commits into
apache:mainfrom
lucasfang:codex/fix-read-ahead-validation
Nov 6, 2026
Merged

lxy-9602 merged 2 commits into
apache:mainfrom
lucasfang:codex/fix-read-ahead-validation

Conversation

@lucasfang

@lucasfang lucasfang commented Nov 6, 2026 •

Copy link
Copy Markdown
Collaborator

Purpose

Fix two existing main-branch bugs identified in the review of #513:

Manually constructed integration-test splits now obtain the actual fixture length through GetFileStatus() instead of using placeholder sizes or reusing an ORC file size for Parquet.

Tests

Five new regression tests cover out-of-file and overflowing ranges, invalid late registrations preserving existing cache state, exact EOF boundaries, unknown file sizes, and cache-disabled LM reads under NONE/RAW/DECODED warmup. The LM test makes every range query fail and checks the complete filtered result. Before the production fix, the expected three regression tests failed; after the fix, all five pass.

Native Debug validation (build-rest, ORC/Parquet/Avro/REST enabled):

  • Common: 1,862 passed; two existing parameterized cases skip because their cache is disabled.
  • Parquet: 240 passed.
  • ORC: 120 passed.
  • Read integration: 303 passed.
  • Write/read with index integration: 252 passed.
  • Formatting, lint and whitespace checks passed for all seven changed files.
cmake --build build-rest --target paimon-common-test paimon-parquet-format-test paimon-orc-format-test -j16
cmake --build build-rest --target paimon-read-inte-test paimon-write-and-read-inte-with-index-test -j16
./build-rest/debug/paimon-common-test
./build-rest/debug/paimon-parquet-format-test
./build-rest/debug/paimon-orc-format-test
ctest --test-dir build-rest -R '^paimon-read-inte-test$' --output-on-failure
ctest --test-dir build-rest -R '^paimon-write-and-read-inte-with-index-test$' --output-on-failure
pre-commit run --files \
  src/paimon/common/utils/read_ahead_cache.cpp \
  src/paimon/common/utils/read_ahead_cache.h \
  src/paimon/common/utils/read_ahead_cache_test.cpp \
  src/paimon/common/reader/prefetch_file_batch_reader_impl.cpp \
  src/paimon/common/reader/late_materializing_file_batch_reader_test.cpp \
  test/inte/read_inte_test.cpp \
  test/inte/write_and_read_inte_with_index_test.cpp
git diff --check

The all-component local build could not finish because of Rust dependency/toolchain incompatibilities (Vortex with Rust 1.67, then Lance/ethnum with installed stable Rust 1.97). The successful native build disables those optional Rust formats. Repository-wide unittest, other integration executables and local sanitizer builds were not run.

API and Format

No changes to installed public API, storage format or protocol. Malformed ranges beyond a known file size now return a validation error before splitting and allocating cache entries. File size zero retains its existing unknown-size behavior.

Documentation

Updated the internal ReadAheadCache constructor and AddRanges() comments to describe validation and the unknown-size convention. No new feature or configuration option.

Generative AI tooling

Generated-by: OpenAI Codex (codex-cli 0.166.0)

@zjw1111 zjw1111 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1 for the fix, but please check and modify the PR description

@lxy-9602 lxy-9602 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1

@lxy-9602
lxy-9602 merged commit b1d5887 into apache:main Nov 6, 2026
16 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants