Repository navigation
branch-4.2: [feature](iceberg) Add iceberg_partition_overwrite_mode session variable (#69010) - #69841
Conversation
### What problem does this PR solve? Issue Number: close apache#29383 Related PR: apache#69010 Problem Summary: A bare Iceberg INSERT OVERWRITE used ReplacePartitions, which can leave data from older partition specs in place. Planner-proven empty inputs also bypassed the transaction needed to clear the target. Add the validated, forwarded iceberg_partition_overwrite_mode session variable, defaulting to static whole-table overwrite, with dynamic preserving the previous scope. Adapt the change to branch-4.2's legacy IcebergTransaction and its existing empty-insert executor hook. Preserve pinned writer metadata, generation leases, authenticated manifest scans, and explicit PARTITION overwrite behavior. Validate concurrent data/deletes against the target branch's base snapshot. ### Release note Bare Iceberg INSERT OVERWRITE now replaces the whole table by default, including data written under older partition specs. Set iceberg_partition_overwrite_mode='dynamic' to keep the previous behavior. An empty-source static overwrite clears the table. Explicit PARTITION clauses retain their existing scope. ### Check List (For Author) - Test: Unit Test / Regression test added and adapted. Modified Java source and tests passed Checkstyle (0 violations) through build.sh --fe. run-regression-test.sh --clean --compile passed, including 47 framework unit tests. FE build and the four selected run-fe-ut.sh suites were attempted but blocked by host-wide kernel OOM kills before test execution. Full Iceberg regression was not run because this worktree has no isolated BE or external REST/S3 fixtures. The new regression output is unchanged from the upstream PR's generated artifact. - Behavior changed: Yes. The default bare Iceberg overwrite scope is static; the dynamic session setting restores the previous scope. - Does this need documentation: Yes. The public session variable and default behavior change need the same documentation as the upstream feature.
|
Thank you for your contribution to Apache Doris. Please clearly describe your PR:
|
|
run buildall |
|
/review |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
Review completed for PR 69841 at the pinned head 2f895ba.
I found no new, verified P0-P3 defect introduced by this PR, so there are no inline findings. The new static bare-overwrite path uses an always-true Iceberg row filter, stages an overwrite even for an empty source, preserves older partition-spec cleanup, and fences concurrent data and delete commits from the selected branch or main snapshot. Static PARTITION overwrites retain precedence. The new session variable is validated, normalized, forwarded, and captured before the write; the empty-insert path now reaches the connector for overwrite statements while dynamic partitioned empty writes remain a no-op.
I independently checked the FE bind and executor path, Iceberg transaction and retained-generation lifecycle, branch selection and commit validation, session-variable reflection/forwarding, changed unit tests, and every changed regression suite. The branch-local concurrency gap in the legacy dynamic ReplacePartitions and explicit static-partition paths is real, but those operation bodies are unchanged from the PR parent; it is recorded as pre-existing hardening context and is not reposted. There were no existing P0/P1 inline comments to confirm. No additional user-provided review focus was supplied.
This review is static-only under the review contract: no build, test, benchmark, runtime, or source-edit validation was run. The review did not converge within the bounded process because coverage remains incomplete for a real catalog static branch overwrite with a branch-local concurrent commit/delete and for an explicit v3/delete-vector whole-table case. Those are coverage limits, not verified defects.
Review did not converge after 3 rounds. This automated review cannot pass the review check.
Round 3 final sweep found no new verified defect, but required end-to-end coverage is incomplete: no real-catalog static branch overwrite with a branch-local concurrent data/delete commit, and no explicit v3/delete-vector whole-table overwrite case. The pre-existing dynamic/static-partition branch isolation gap was verified unchanged from the parent.
|
PR approved by anyone and no changes requested. |
github-actions
Bot
added
the
approved
|
PR approved by at least one committer and no changes requested. |
### 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
pick #69010