hubgeter opened a new pull request, #70517: URL: https://github.com/apache/doris/pull/70517
### Background Iceberg tables retain partition specifications as partitioning evolves. The `rewrite_data_files` procedure exposes `output-spec-id` to select the partitioning of replacement files without changing the table's default specification. ### What problem does this PR solve? Issue Number: None Related PR: #56413 (introduced the procedure) PR Type: Bugfix Problem Summary: The procedure rejects valid spec 0, accepts unknown positive IDs, and discards the requested output specification before writing. Resolve the optional ID against table metadata and carry one immutable writer context through the shared rewrite transaction, sink construction, and commit conversion. Replacement files now use the selected specification and matching partition tuples; omitted IDs resolve to the planning-time default. ### Details - **Change map:** 151 changed lines of production code and 508 changed lines of tests; no generated files or documentation changes. - **Frequency:** Explicit spec 0 is always rejected before this fix. Positive IDs are ignored whenever a rewrite reaches execution. - **Root cause:** A positive-only parser rejects 0, planner parameters drop the ID, and both the rewrite sink and commit decoder independently use the current table specification. - **Why existing tests missed it:** Planner fixtures explicitly treated the output ID as unused, and sink/transaction coverage checked current-spec writes without carrying a procedure-selected historical spec across independent group scopes. - **Fix:** Validate IDs in the nonnegative integer range and reject absent specs during planning, including empty tables. Reuse statement scope and the existing per-table transaction interface to hand one pinned context to every group. Use its partition definition for both BE metadata and FE commit conversion, including nonzero unpartitioned specs. Preserve existing schema/table generation checks and reject sort-order drift before a group can use a differently bound sorter. - **Other paths and branches:** Existing sink Thrift fields and SPI methods are reused. The same defect is present on `branch-4.2` at `1bb4a49bddc8a505003d2067cc56f0dc3432f23f`; a separate semantic backport is recommended because that branch uses the legacy FE Iceberg implementation. - **Behavior change:** Valid spec 0 and compatible historical specifications are honored; unknown or out-of-range IDs fail. The table default is preserved. Incompatible schema changes or a changed sort definition require retrying the statement. ### Testing - Passed locally: 389 targeted FE unit tests through `run-fe-ut.sh`, covering action validation, planner, procedure dispatch, metadata, transaction, write provider, statement scope, and write transaction coverage. - The new tests cover a historical day spec with mixed input specs and independent group scopes, spec 0, a nonzero unpartitioned spec, a changed default after planning, an unknown spec on an empty table, a dropped partition source, and schema/sort evolution. They inspect real Iceberg commit metadata using simulated BE commit fragments. - Passed locally: Iceberg connector Checkstyle, 0 violations. - Standard `build.sh --fe` build is in progress. - Added SQL regression coverage for spec 0, unknown IDs, and invalid ID ranges. Live BE/SQL regression was not run in this worktree. ### Notes for Reviewers The new coverage checks sink metadata and committed DataFiles; it does not execute BE file writers. Start with the procedure-to-transaction handoff and the context used by `buildRewriteSink` and `convertCommitDataToFilesToAdd`. ### Release note Iceberg `rewrite_data_files` now honors `output-spec-id`, including spec 0 and compatible historical partition specifications, and rejects unknown or out-of-range IDs. Omitting the option uses the specification selected during planning. ### Check List (For Author) - Behavior changed: Yes (see Details). - Does this need documentation: Yes. Update the parameter reference to describe the planning-time current-spec default and the nonnegative integer range; documentation follow-up is planned. - Pre-submission status: Partially verified: standard FE build is running; live BE/SQL regression is pending. ### AI Disclosure - Tool: Codex - Generated: Code, tests, and this description. - Human review: Pending. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
