feat: support parquet page row count limit - #459
Open
lucasfang wants to merge 3 commits into
Open
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
This PR adds support for configuring a Parquet data page row count limit via Paimon’s Parquet writer properties, alongside related dependency patch updates and test adjustments.
Changes:
- Add a new Parquet write option
parquet.page.row.count.limitand plumb it intoParquetWriterBuilder. - Extend
ParquetWriterBuilderunit tests to cover the row count limit property. - Improve
LoggerTest.TestGlogWritesLogFileToDiskcleanup by usingScopeGuardto restore process-wide glog state. - Expand the bundled Arrow/Parquet patch (
cmake_modules/arrow.diff) to introduce/adjust Parquet writer properties (including the row count limit) and other Arrow/Parquet behaviors.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| src/paimon/format/parquet/parquet_writer_builder.cpp | Reads the new option and sets Parquet writer data_page_row_count_limit. |
| src/paimon/format/parquet/parquet_writer_builder_test.cpp | Adds assertions for default and configured data_page_row_count_limit. |
| src/paimon/format/parquet/parquet_format_defs.h | Defines the new option key parquet.page.row.count.limit and documents its semantics. |
| src/paimon/common/logging/logging_test.cpp | Uses ScopeGuard to reliably restore glog global flags/destinations after the test. |
| cmake_modules/arrow.diff | Updates the vendored Arrow/Parquet patch to add row count limit support and additional related/unrelated changes. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+1
to
+5
| diff --git a/cpp/src/arrow/io/interfaces.h b/cpp/src/arrow/io/interfaces.h | ||
| index b36c38c6d4..f974a33073 100644 | ||
| --- a/cpp/src/arrow/io/interfaces.h | ||
| +++ b/cpp/src/arrow/io/interfaces.h | ||
| @@ -210,7 +210,7 @@ class ARROW_EXPORT InputStream : virtual public FileInterface, virtual public Re |
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
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.
Purpose
Linked issue: close #xxx
This change adds the write option
parquet.page.row.count.limitto the Parquetformat layer, aligning with parquet-mr's
parquet.page.row.count.limit(PARQUET-1414, parquet-mr 1.11.0) in both semantics and default value (20000):
cmake_modules/arrow.diff): extendsparquet::WriterPropertieswith
data_page_row_count_limit(defaultDEFAULT_DATA_PAGE_ROW_COUNT_LIMIT = 20000).TypedColumnWriterImplnow finishes the current data page wheneither the estimated encoded size reaches
parquet.page.sizeor the bufferedrow count reaches the limit, whichever comes first — the same rule parquet-mr
uses. The limit is checked at write-batch granularity.
parquet_format_defs.handplumbs it through
ParquetWriterBuilder::PrepareWriterPropertiesinto theArrow
WriterProperties::Builder.Bounding rows per page keeps the column index (page index) fine-grained, so
page-level predicate pushdown stays effective on pages that would otherwise
hold a huge number of highly-compressible rows.
Behavior notes (default changed from "unlimited" to 20000, matching Java):
reduces writer memory (e.g. the all-null case in
TestMemoryControldropsfrom ~20 MB to ~0.36 MB peak).
parquet.writer.max.memory.usepeak can transiently reach ~2.2x thebudget: the budget is only checked between batches, and with row-count
splitting the budget is mostly held as finished page buffers, which
BufferedPageWritercopies into its in-memory sink when the row group isflushed (2x plus one batch is the theoretical bound regardless of encoding).
TestMemoryControlwas updated accordingly with the measured rationale incomments.
The commit also hardens
logging_test.cpp: glog global state (log destinationand flags) is now restored via
ScopeGuardon every exit path, and the<program>.INFOsymlink is disabled so temporary-directory cleanup cannotbreak on a dangling link.
Tests
Unit tests (all run locally and passing):
ParquetWriterBuilderTest.DefaultPrepareWriterProperties— asserts thedefault
data_page_row_count_limitis 20000.ParquetWriterBuilderTest.PrepareWriterProperties— setsparquet.page.row.count.limit=40000and asserts it overrides the default.TestColumnWriter.WriteDataPagesSplitOnRowCountLimit(inarrow.diff) — verifies pages split exactly at the row-count limit for bothflat and repeated columns, and repeated columns do not split records across
pages.
ParquetFormatWriterTest.TestMemoryControl— re-baselined for the newdefault: all-null runs assert the writer stays below the memory budget;
non-null runs assert the budget triggers row-group splitting with the peak
bounded by 2.5x.
API and Format
include/paimon/.parquet.page.row.count.limit(write path,default 20000, aligned with parquet-mr).
finer for row-count-bound pages, which increases page index entries per
column chunk.
Documentation
No standalone documentation change; the option key and its semantics are
documented in
parquet_format_defs.hnext to the definition.Generative AI tooling
Generated-by: Qoder