Skip to content

Read required column batches from the cursor position - #205

Closed
quinnj wants to merge 1 commit into
masterfrom
maintenance/parquet-required-columns
Closed

quinnj wants to merge 1 commit into
masterfrom
maintenance/parquet-required-columns

Conversation

@quinnj

@quinnj quinnj commented Oct 4, 2026

Copy link
Copy Markdown
Member

Reading required (non-nullable) Parquet columns through BatchedColumnsCursor, Parquet.Table, or read_parquet reads past the populated page buffer. With bounds checking, five required scalar columns fail with BoundsError even though the row cursor returns every value correctly.

Copy each batch from the column cursor's current value position. The previous index added to the page's populated-value count instead. Existing page and row-group advancement remains in use.

Validation: unchanged source passes four row/column controls and fails ten batched/table controls on actual Julia 1.3.1 and 1.13.1. The fix passes all 14 controls and 560 permanent checks on both versions. These cover required Int32, Int64, Float64, Boolean and UTF-8 columns; plain/dictionary encoding; partial row ranges; batches spanning pages and row groups; buffer reuse; threads; and both public table factories. Committed PyArrow 16.1.0 fixtures are independently verified by PyArrow 25.0.1 and DuckDB 1.5.6. Python is only needed to regenerate fixtures.

Both full suites pass with four threads and bounds checking: 5,148 checks on Julia 1.3.1 and 5,134 on Julia 1.13.1. An independent agent privately reviewed all seven changed files and the page-position/advancement paths and found no blocker. git diff --check passes. No package dependencies, exports, or version changes.

Co-authored by Codex

AI disclosure: This work was prepared with assistance from OpenAI Codex.

The page offset counts populated values, so adding an index reads beyond
the page. Use the current value position, matching page advancement.
Cover required scalar types and both public table readers with external
plain and dictionary fixtures across row groups and batch boundaries.

AI disclosure: This work was prepared with assistance from OpenAI Codex.
@codecov

codecov Bot commented Oct 4, 2026 •

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 87.96%. Comparing base (c67ff83) to head (cb2cde8).

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #205      +/-   ##
==========================================
+ Coverage   86.84%   87.96%   +1.12%     
==========================================
  Files          10       10              
  Lines        1604     1604              
==========================================
+ Hits         1393     1411      +18     
+ Misses        211      193      -18     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@quinnj

quinnj commented Oct 4, 2026

Copy link
Copy Markdown
Member Author

Closing this standalone change to keep further Parquet fixes and improvements inside the active rewrite PR, as requested by the maintainer. The local reproduction and regression evidence are preserved for reference when reviewing that rewrite.

AI disclosure: This work was prepared with assistance from OpenAI Codex.

@quinnj quinnj closed this Oct 4, 2026
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.

1 participant