branch-4.2: [fix](iceberg) Commit every Iceberg file under the table's own location #69392 by xylaaaaa · Pull Request #70173 · apache/doris

xylaaaaa · 2026-11-04T03:54:45Z

What problem does this PR solve?

Issue Number: None

Related PR: #69392

Problem Summary:
Backport #69392 to branch-4.2. Iceberg DELETE/UPDATE/MERGE could record position-delete and Puffin files under BE's normalized s3:// address even when the table uses oss://, s3a://, or abfss://. These metadata paths can prevent Doris and other engines from reading the files.

BE now derives physical I/O paths and raw metadata paths from one output location for data files, position deletes, and deletion vectors. FE registers the raw locations from the actual bound sinks and validates the entire collected file list before staging INSERT/overwrite, DELETE, MERGE, or rewrite updates.

Release-branch adaptation:

Use the existing typed Iceberg transaction and planner sinks. Validate at transaction completion rather than in the report RPC, since these branches do not have [fix](iceberg) Harden external-write lifecycle, OCC, memory, and file ownership #66348's report ACK mechanism.
Register rewrite group locations after binding the shared transaction; preserve empty static partition overwrites, which execute without a BE sink.
Preserve the branch's existing failed-file cleanup behavior. This backport does not add ACK-based ownership transfer or rollback file deletion.
No wire-format changes; the Thrift changes are comments only.
Adapt the branch-specific Iceberg nanosecond Parquet writer test to the new output-location object.

Release note

Fix Iceberg files being committed with normalized storage addresses instead of their raw table data locations. Reject out-of-location file paths before staging Iceberg updates. Upgrade BE before FE: an older BE that reports normalized delete-file paths will be rejected by the upgraded FE. Failed writes can still leave uncommitted files under the release branch's existing cleanup behavior.

Check List (For Author)

Test:
- Unit Test: 111 FE tests passed via run-fe-ut.sh --run (117 discovered; 6 pre-existing disabled tests skipped), covering transactions, all four completion paths, raw/normalized and directory-prefix rejection, sink registration, empty static overwrite, rewrite groups, and SQL failure propagation.
- IcebergWriteTransactionCoverageTest: all 11 cases passed after adapting the 4.2 fixture. The 4 IcebergWriteKnownBugsTest cases remain disabled as before.
- FE packaging and ASAN BE build passed via build.sh; the generated BE binary passed --version. FE UI was skipped. Local Clang 22 compatibility options and LLVM archiving tools were used for baseline macOS toolchain issues and are not part of this PR.
- Java Checkstyle passed for the final changed files; C++ clang-format 16 checks passed.
- BE unit tests: no cases executed locally. The complete ASAN UT build stops in the unchanged ann_index_edge_case_test.cpp because set(long) is ambiguous on macOS. The changed Iceberg production translation units compiled successfully; validate the complete test suite in CI.
- Iceberg regression: cases are included; not run locally because the external fixture/Docker daemon is unavailable.
- C++ static analysis was attempted; the full run is not green due to existing macro/NOLINT/complexity diagnostics. No broad suppressions or environment changes are included.
Behavior changed:
- Yes: metadata paths use raw locations, and invalid reported paths fail the statement.
Does this need documentation?
- No.

Check List (For Reviewer who merge this PR)

Confirm the release note
Confirm test cases
Confirm document
Add branch pick label

### What problem does this PR solve? Issue Number: None Related PR: apache#69392 Problem Summary: Iceberg delete and Puffin files were committed using the normalized storage URI instead of the table's original data location. Other engines could resolve those paths with the wrong file system. Carry the write and metadata locations together for data, position-delete, and Puffin files, join trailing separators consistently, and retain the write path for cleanup. Backport the BE path fix and relevant tests to branch-4.2. Adapt writer tests to the release branch interfaces and add planner coverage for raw versus normalized paths. Omit FE commit-location rejection and the external-file report ACK prerequisite from apache#66348, as those mechanisms are not on this branch. ### Release note Preserve the table's original URI in newly written Iceberg file metadata, including position deletes and deletion vectors, and handle trailing slashes in custom data locations consistently. ### Check List (For Author) - Test: Unit Test - 16 FE planner sink tests passed; clang-format 16, Java Checkstyle, patch comparison, and whitespace checks passed. BE UT build was attempted but blocked by macOS/Clang baseline incompatibilities. clang-tidy reported macro/baseline lint diagnostics. Iceberg regression was not run because the external fixture was unavailable. - Behavior changed: Yes - newly committed file paths retain the table's URI; sinks missing required locations fail before creating files. - Does this need documentation: No Adapted partial backport of commit b78c656.

### What problem does this PR solve? Issue Number: None Related PR: apache#69392 Problem Summary: Release branches accept Iceberg file reports without validating their raw sink locations. A writer reporting a normalized s3:// path for an oss://, s3a:// or abfss:// location can publish an unreadable file address. Register the locations from the bound sinks and validate the entire collected file list before staging INSERT, overwrite, DELETE, MERGE or rewrite updates. Propagate failure through the existing transaction completion path. Preserve empty static overwrites and register rewrite groups on their shared transaction. This adapts the FE guard without the external-report ACK and cleanup changes. ### Release note Reject Iceberg writes whose reported files are outside their sink's raw locations. Upgrade BE before FE when applying the location fix. Failed-file cleanup retains the release branch's existing behavior. ### Check List (For Author) - Test: 83 focused FE unit tests passed via run-fe-ut.sh; Maven Checkstyle passed. - Unit Test: transaction staging, path rejection, executor registration, empty static overwrite, SQL error propagation and existing sink tests. - Behavior changed: Yes; invalid file paths fail before Iceberg updates are staged. - Does this need documentation: No

hello-stephen · 2026-11-04T03:54:59Z

Thank you for your contribution to Apache Doris.
Don't know what should be done next? See How to process your PR.

Please clearly describe your PR:

What problem was fixed (it's best to include specific error reporting information). How it was fixed.
Which behaviors were modified. What was the previous behavior, what is it now, why was it modified, and what possible impacts might there be.
What features were added. Why was this function added?
Which code was refactored and why was this part of the code refactored?
Which functions were optimized and what is the difference before and after the optimization?

xylaaaaa · 2026-11-04T06:22:42Z

run buildall

Copilot

Copilot was unable to review this pull request because the user who requested the review has insufficient remaining quota. Try requesting a review at a lower effort level.

### What problem does this PR solve? Issue Number: None Related PR: apache#69392 Problem Summary: The 4.2 coverage fixture bypasses planning and constructs Iceberg transactions directly. It never registers a raw sink location, so the backported validation rejects its s3://bucket/events files before the existing transaction assertions run. Ten of eleven transaction coverage tests failed. Register the matching table, delete, or merge sink in beginWrite, adapting upstream 52c04ac to the release branch. Also add risk R19, case W20, and the file-location suite to the existing P0 coverage matrix. ### Release note None ### Check List (For Author) - Test: Unit Test; run-fe-ut.sh executed 117 tests across 15 classes, with 111 passing, 6 pre-existing disabled tests skipped, and no failures or errors. The transaction coverage class passed all 11 tests; all 4 known-bug cases remain disabled. Maven Checkstyle and git diff --check passed. - Behavior changed: No - Does this need documentation: Yes; update the requested existing coverage matrix in this commit.

xylaaaaa · 2026-11-04T06:59:20Z

run buildall

hello-stephen · 2026-11-04T07:23:53Z

Cloud UT Coverage Report

Increment line coverage 🎉

Increment coverage report
Complete coverage report

Category	Coverage
Function Coverage	76.44% (2005/2623)
Line Coverage	64.94% (36139/55652)
Region Coverage	52.82% (34524/65367)
Branch Coverage	55.72% (10766/19322)

### What problem does this PR solve? Issue Number: None Related PR: apache#69392, apache#70173, apache#69841, apache#70185 Problem Summary: Merge the latest branch-4.2 into the Iceberg file-location backport. Preserve the session partition-overwrite mode together with bound-sink location registration. Adapt the newly added Parquet writer test to IcebergOutputLocation, and update overwrite fixtures for explicit modes, retained snapshots, and registered commit locations. Check path acceptance and rejection for both static and dynamic overwrite modes. ### Release note None ### Check List (For Author) - Test: Unit Test via run-fe-ut.sh --run (16 related FE classes; 122 passed, 6 pre-existing disabled cases skipped, zero failures or errors); Java Checkstyle; clang-format 16; Parquet test translation-unit syntax check via clang-tidy and the existing ASAN compilation database. BE unit tests and external regressions are delegated to CI. - Behavior changed: No; preserve both the release branch's overwrite behavior and this PR's file-location validation. - Does this need documentation: No

xylaaaaa · 2026-11-07T15:47:47Z

run buildall

xylaaaaa and others added 2 commits November 4, 2026 10:59

xylaaaaa marked this pull request as ready for review November 4, 2026 06:22

xylaaaaa requested a review from yiguolei as a code owner November 4, 2026 06:22

Copilot AI balanced review requested due to automatic review settings November 4, 2026 06:22

Copilot AI reviewed Nov 4, 2026

xylaaaaa changed the title ~~[fix](iceberg) Commit files under table locations (branch-4.2)~~ branch-4.2: [fix](iceberg) Commit every Iceberg file under the table's own location #69392 Nov 4, 2026

Navigation Menu

Repository navigation

branch-4.2: [fix](iceberg) Commit every Iceberg file under the table's own location #69392 - #70173

branch-4.2: [fix](iceberg) Commit every Iceberg file under the table's own location #69392#70173
xylaaaaa wants to merge 4 commits into
apache:branch-4.2from
xylaaaaa:codex/pick-69392-4.2

xylaaaaa commented Nov 4, 2026 •

edited

Loading

Uh oh!

hello-stephen commented Nov 4, 2026

Uh oh!

xylaaaaa commented Nov 4, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

xylaaaaa commented Nov 4, 2026

Uh oh!

hello-stephen commented Nov 4, 2026

Uh oh!

xylaaaaa commented Nov 7, 2026

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

xylaaaaa commented Nov 4, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What problem does this PR solve?

Release note

Check List (For Author)

Check List (For Reviewer who merge this PR)

Uh oh!

hello-stephen commented Nov 4, 2026

Uh oh!

xylaaaaa commented Nov 4, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Uh oh!

xylaaaaa commented Nov 4, 2026

Uh oh!

hello-stephen commented Nov 4, 2026

Cloud UT Coverage Report

Uh oh!

xylaaaaa commented Nov 7, 2026

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

xylaaaaa commented Nov 4, 2026 •

edited

Loading