Skip to content

[python] Support row-count based file rolling for data-evolution append tables - #8863

Merged
JingsongLi merged 6 commits into
apache:masterfrom
XiaoHongbo-Hope:de_row_num_rolling
Jul 28, 2026
Merged

[python] Support row-count based file rolling for data-evolution append tables#8863
JingsongLi merged 6 commits into
apache:masterfrom
XiaoHongbo-Hope:de_row_num_rolling

Conversation

@XiaoHongbo-Hope

Copy link
Copy Markdown
Contributor

Purpose

#8739 added row-count based data file rolling (target-file-row-num) on the core side. PyPaimon file-store writers previously did not support it and failed fast whenever the option was enabled.

This PR implements target-file-row-num rolling for data-evolution append tables in PyPaimon.

Changes

  • DataWriter._check_and_roll_if_needed: roll a file when it reaches target-file-row-num rows or target-file-size, whichever comes first (exact slicing, mirroring the format-table path). Defaults to the max long (disabled), so size-based rolling is unchanged when the option is unset.
  • FileStoreWrite._create_data_writer: narrow the fail-fast so the option is permitted only for data-evolution append tables (i.e. AppendOnlyDataWriter — not primary-key, blob or vector writers, which override rolling and remain unsupported for now).

Tests

data_evolution_row_rolling_test.py:

  • rolls when the row count exceeds the limit (10 rows / limit 3 → files of 3,3,3,1);
  • exact multiple rolls evenly;
  • below the limit stays a single file;
  • unset option does not roll by rows;
  • a non-DE table still fails fast.

@XiaoHongbo-Hope
XiaoHongbo-Hope marked this pull request as ready for review July 27, 2026 13:43

@leaves12138 leaves12138 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.

I found a regression in the existing shard-update path; details are inline.

# Row-count trigger: keep at most target_file_row_num rows per file.
split_row = num_rows
if num_rows > self.target_file_row_num:
split_row = self.target_file_row_num

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.

This also enables row-count rolling for ShardTableUpdator, which constructs AppendOnlyDataWriter directly. That path only overrides target_file_size and SingleWriter.end() requires exactly one output file. Repro: create a data-evolution table with one 5-row file, alter target-file-row-num to 2, then run new_shard_updator; the writer produces three files and fails with Should have one file. The base revision produces one 5-row file. Please disable row rolling in ShardTableUpdator as already done by TableUpdateByRowId (or otherwise scope this logic to ordinary append writes), and add a regression test for this path.

ShardTableUpdator constructs an AppendOnlyDataWriter directly and only
overrode target-file-size. With target-file-row-num set, the writer
would roll the shard into multiple files and SingleWriter.end() failed
with "Should have one file." Disable row-count rolling as well, mirroring
FileStoreWrite.disable_rolling() used by the row-id update path.

Adds a regression test that alters target-file-row-num on a
data-evolution table and runs a shard update over a single 5-row file.

@JingsongLi JingsongLi 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.

+1

@JingsongLi
JingsongLi merged commit 3120ac7 into apache:master Jul 28, 2026
15 of 20 checks passed
XiaoHongbo-Hope added a commit to XiaoHongbo-Hope/paimon that referenced this pull request Jul 28, 2026
The regression test added in apache#8863 left two blank lines before the
method, tripping flake8 E303 on the whole-tree lint. Use a single
blank line.
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