Skip to content

fix(parquet): limit rows per data page to prevent overflow - #812

Open
olemon111 wants to merge 1 commit into
bytedance:mainfrom
olemon111:fix_parquet_page_row_limit
Open

fix(parquet): limit rows per data page to prevent overflow#812
olemon111 wants to merge 1 commit into
bytedance:mainfrom
olemon111:fix_parquet_page_row_limit

Conversation

@olemon111

@olemon111 olemon111 commented Jul 31, 2026

Copy link
Copy Markdown
Collaborator

What problem does this PR solve?

Bolt's Parquet writer could generate a file that completed successfully during
write but failed later during read with an error like:

Expected -1509661479 values in column chunk at offset 32065246
but got 0 values instead over 0 pages ending at file offset 32065223

The invalid negative value came from an overflowing DataPage count. The writer
previously used the estimated encoded byte size as the main DataPage flush
condition. For highly compressible data, especially dictionary-encoded
low-cardinality BYTE_ARRAY values and all-null columns, billions of logical
values could accumulate in one page while the encoded buffer remained below the
configured byte limit.

Issue Number: close #800

Type of Change

  • 🐛 Bug fix (non-breaking change which fixes an issue)
  • ✨ New feature (non-breaking change which adds functionality)
  • 🚀 Performance improvement (optimization)
  • ⚠️ Breaking change (fix or feature that would cause existing functionality to change)
  • 🔨 Refactoring (no logic changes)
  • 🔧 Build/CI or Infrastructure changes
  • 📝 Documentation only

Description

This PR:

  • adds WriterProperties::max_rows_per_page with a default of 20,000 rows,
    matching the current Apache Arrow C++, Arrow Rust, and parquet-java default;
  • exposes the limit as Bolt WriterOptions::maxRowsPerDataPage and passes it
    into the Arrow-derived Parquet writer properties;
  • makes block-size-based buffered writes honor the row count carried by an
    existing custom flushPolicyFactory, without adding another
    WriterOptions field; this lets existing integrations enforce both row and
    compressed-byte RowGroup limits;
  • changes all relevant column-writing paths to split work according to both
    the internal write-batch size and the remaining row capacity of the current
    DataPage;
  • preserves record boundaries for repeated/nested columns where page-index or
    DataPageV2 semantics require record-aligned pages;
  • flushes a DataPage when either the encoded byte threshold or row threshold
    is reached;
  • validates DataPage V1/V2 counts before narrowing them to the Parquet
    page-header int32 fields, providing a fail-fast safety net;
  • adds property validation and regression coverage for:
    • default and custom row limits;
    • invalid zero/negative limits;
    • all-null optional columns with DataPage V1 and V2;
    • ZSTD-compressed, dictionary-encoded, low-cardinality BYTE_ARRAY columns;
    • repeated columns and record-boundary page splitting;
    • the high-level Bolt writer round trip.

The incoming Spark/Gluten RecordBatch split is intentionally not used as the
correctness boundary. Multiple input batches can be appended to the same open
DataPage, so the limit must be enforced inside ColumnWriter.

The 20,000-row default follows existing community practice rather than
introducing a Bolt-specific value. Arrow added an independent row limit for the
same byte-only-page-size problem, Arrow C++ documents max_rows_per_page as
20K rows by default, and parquet-java exposes the same default as
DEFAULT_PAGE_ROW_COUNT_LIMIT:

Performance Impact

  • No Impact: This change does not affect the critical path (e.g., build system, doc, error handling).

  • Positive Impact: I have run benchmarks.

    Click to view Benchmark Results
    Paste your google-benchmark or TPC-H results here.
    Before: 10.5s
    After:   8.2s  (+20%)
    
  • Negative Impact: Explained below (e.g., trade-off for correctness).

Correctness now takes priority over keeping an unbounded number of logical rows
in one DataPage. For columns that previously produced extremely large logical
pages, this change creates more pages. The possible costs are additional page
headers/statistics, more compression finalizations, a larger footer/page index,
and some extra CPU during full scans.

The limit can also improve peak memory bounds, cache behavior, and page-index
predicate pruning. Ordinary columns that reach the byte-size threshold first
are largely unaffected.

Release Note

Please describe the changes in this PR

Release Note:

Release Note:
Fix the Parquet writer so highly compressible columns cannot overflow DataPage
row/value count fields. DataPages are now limited to 20,000 rows by default,
and page-header counts are validated before serialization.

Checklist (For Author)

  • I have added/updated unit tests (ctest).
  • I have verified the code with local build (Release/Debug).
  • I have run clang-format / linters.
  • (Optional) I have run Sanitizers (ASAN/TSAN) locally for complex C++ changes.
  • No need to test or manual test.

Breaking Changes

  • No

  • Yes (Description: ...)

    Click to view Breaking Changes
    Breaking Changes:
    - Description of the breaking change.
    - Possible solutions or workarounds.
    - Any other relevant information.
    

Copilot AI review requested due to automatic review settings July 31, 2026 08:04

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

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.

[Bug] Parquet writer can overflow DataPage row/value counts for highly compressible columns

2 participants