Skip to content

[fix](iceberg) Pass rewrite file size to each writer - #70510

Draft
hubgeter wants to merge 1 commit into
apache:masterfrom
hubgeter:fix/iceberg-rewrite-writer-target
Draft

hubgeter wants to merge 1 commit into
apache:masterfrom
hubgeter:fix/iceberg-rewrite-writer-target

Conversation

@hubgeter

@hubgeter hubgeter commented Nov 12, 2026 •

Copy link
Copy Markdown
Contributor

Background

Iceberg rewrite_data_files selects and groups input files, then runs an INSERT-SELECT for each group under one shared rewrite transaction. Each task clones the caller's session, and the backend writer reads its rollover target from the task's query options.

What problem does this PR solve?

Issue Number: None
Related PR: #64304 (introduced the regression), #56413 (original implementation)
PR Type: Bugfix

Problem Summary:

The procedure's target-file-size-bytes reaches file selection but is lost before the output write. For example, a 128 MiB procedure target with the default session value still uses the backend's 1 GiB writer fallback. Carry the connector-resolved value, including the 512 MiB action default, with every rewrite group and apply it only to that task's cloned session before planning the write.

Details

  • Frequency: Rewrites that produce groups and whose effective action target differs from the inherited writer option or backend fallback.
  • Root cause: The connector SPI migration omitted the legacy per-task writer-target assignment. Neutral groups carried paths and statistics, while tasks inherited the original session unchanged.
  • Why existing tests missed it: Planning tests checked file paths and statistics; task tests covered cancellation without checking the serialized writer options.
  • Fix: Add immutable connector-resolved task-session overrides to rewrite groups and apply them through the existing session-variable setter. Keep procedure parsing and default resolution inside the connector. Advance the connector API to 31.0 and record the rewrite-group surface.
  • Other paths and branches: Ordinary INSERT keeps its existing session/fallback behavior. branch-4.2 at 1bb4a49bddc8a505003d2067cc56f0dc3432f23f already passes the action target through its legacy executor and sets it on the task session, so this fix needs no 4.2 backport. No Thrift field or BE implementation changes are needed.

Testing

  • ./build.sh --fe -j4: passed locally, including FE packaging, Checkstyle and connector architecture gates.
  • run-fe-ut.sh: all 154 SPI tests, 53 related connector tests, 12 rewrite-driver tests and 7 rewrite-task tests passed locally (226 cases across 42 classes). Both SPI surface checks passed.
  • Final Checkstyle passed for the changed FE modules and the latest task test fixture.
  • Removing the task override application made the query-options regression test fail: 134217728 expected, 0 observed. After restoring the committed implementation, all seven task tests passed again.

The tests check the actual SessionVariable.toThrift() field and its presence bit, default and explicit values, multiple groups, values above the 32-bit range, immutable settings, invalid input and concurrent task-session isolation. Physical output-file distributions and end-to-end SQL rewrite have not been measured locally; file sizes remain approximate rollover targets.

Release note

Iceberg rewrite_data_files now applies target-file-size-bytes, including its default 512 MiB value, to output writers. Output file counts may change when the effective target differs from the previously inherited session limit or backend fallback.

Check List (For Author)

  • Behavior changed: Yes (see Problem Summary and Release note).
  • Does this need documentation: No; this restores the documented procedure behavior.
  • Pre-submission status: Partially verified: local FE build, Checkstyle and 226 targeted unit tests passed. End-to-end SQL and remote CI remain pending.

AI Disclosure

  • Tool: Codex (GPT-6).
  • Generated: Code, tests and this description.
  • Human review: Pending.

### What problem does this PR solve?

Issue Number: None

Related PR: apache#64304 (introduced the regression), apache#56413 (original implementation)

Problem Summary: Rewrite planning resolves target-file-size-bytes, but the connector group drops it and each task inherits the caller's writer target. Carry validated task-session settings with every rewrite group and apply them to its private session before planning. Preserve the effective procedure default and the caller's original settings, and advance the connector API to 31.0.

### Release note

Iceberg rewrite_data_files now applies its target-file-size-bytes option, including the default 512 MiB value, to output writers. Output file counts may change when this differs from the previously inherited writer limit.

### Check List (For Author)

- Test: Unit Test. FE build, final Checkstyle, all 154 SPI tests, 53 connector tests and 12 rewrite-driver tests passed locally. Task-session verification is in progress at commit time; SQL rewrite and BE file-distribution tests have not run locally.
- Behavior changed: Yes, rewrite writers use the procedure's effective target without modifying the caller's session.
- Does this need documentation: No, restore documented behavior.
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