Repository navigation
[fix](iceberg) Execute empty static overwrite and stabilize info_schema timezone test - #65105
Open
suxiaogang223 wants to merge 4 commits into
Open
suxiaogang223 wants to merge 4 commits into
suxiaogang223 wants to merge 4 commits into
Conversation
Contributor
|
Thank you for your contribution to Apache Doris. Please clearly describe your PR:
|
suxiaogang223
marked this pull request as ready for review
July 1, 2026 09:26
suxiaogang223
requested review from
924060929,
englefly,
morrySnow and
starocean999
as code owners
July 1, 2026 09:26
Member
Author
|
run buildall |
Contributor
TPC-H: Total hot run time: 29686 ms |
Contributor
TPC-DS: Total hot run time: 174611 ms |
Contributor
ClickBench: Total hot run time: 25.17 s |
Contributor
FE Regression Coverage ReportIncrement line coverage |
Member
Author
|
/review |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
I found one correctness issue in the empty Iceberg overwrite execution gate.
Critical checkpoints: the PR goal is focused and the static-partition empty overwrite path is covered at the transaction-unit level, but the command-level empty-source gate still misses an existing parallel Iceberg overwrite semantic for unpartitioned tables. No new concurrency, locking, config, persistence format, FE-BE protocol, or rolling-upgrade compatibility concern was found. The changed regression test keeps the rowsets query deterministic enough for this timezone case. Tests were not run locally because this checkout is missing .worktree_initialized and thirdparty/installed, including thirdparty/installed/bin/protoc.
User focus: no additional user-provided focus points were supplied.
Subagent conclusions: optimizer-rewrite reported OPT-1, which was merged as a duplicate confirmation of MAIN-1 and became the inline comment. tests-session-config reported no candidate findings. Convergence round 1 ended with both optimizer-rewrite and tests-session-config replying NO_NEW_VALUABLE_FINDINGS for the same ledger/comment set.
### What problem does this PR solve? Issue Number: None Related PR: apache#65105 Problem Summary: Empty-source Iceberg overwrite inserts are skipped before reaching the transaction layer unless the overwrite uses static partitions. The transaction layer already supports clearing unpartitioned Iceberg tables when overwrite has no pending files, but the executor gate prevented SQL-level unpartitioned overwrites from using that path. This change allows empty overwrite execution for unpartitioned Iceberg tables while preserving the no-op behavior for empty dynamic overwrites on partitioned tables. ### Release note Fix empty-source Iceberg overwrite for unpartitioned tables. ### Check List (For Author) - Test: Unit Test - ./run-fe-ut.sh --run org.apache.doris.nereids.trees.plans.commands.insert.IcebergInsertExecutorTest - Behavior changed: Yes. Empty-source INSERT OVERWRITE on unpartitioned Iceberg tables now reaches the overwrite commit path and clears the table. - Does this need documentation: No
Member
Author
|
run buildall |
1 similar comment
Member
Author
|
run buildall |
zhangstar333
approved these changes
Jul 8, 2026
github-actions
Bot
added
the
approved
Contributor
|
PR approved by at least one committer and no changes requested. |
Contributor
TPC-H: Total hot run time: 29882 ms |
englefly
approved these changes
Jul 8, 2026
Contributor
TPC-DS: Total hot run time: 181648 ms |
Contributor
ClickBench: Total hot run time: 25.11 s |
Contributor
FE Regression Coverage ReportIncrement line coverage |
Merged
4 of 14 tasks
morningman
added a commit
that referenced
this pull request
Oct 28, 2026
…nner proves empty (#69291) ### What problem does this PR solve? Issue Number: None Related PR: #69010, #62344 Problem Summary: **In short.** This ports the test of branch-4.1 #62344 to master and adds the tests that master's broader fix, #69010, does not have: an `INSERT OVERWRITE` whose source the planner proves empty must still replace its target with nothing. Master behaves correctly since #69010, but none of its tests reaches the code path #69010 changed, so taking that change back out keeps every test green. The tests added here fail without it. **Background.** When the planner folds the source of an `INSERT OVERWRITE` to an empty relation (`LIMIT 0`, `WHERE false`, a contradiction) and the sink needs no exchange below it, the sink sits directly on `PhysicalEmptyRelation`. `InsertIntoTableCommand` then treats the insert as empty, and no backend runs it. An empty `INSERT` has nothing to commit, but an empty overwrite still has to replace the table, or the static partition, with nothing. Before #69010, `requiresTransaction()` was false for every empty insert, so the overwrite never reached the connector: the statement returned OK and every row stayed. branch-4.1 #62344 fixed this for an Iceberg static partition overwrite only (`IcebergInsertExecutor.shouldExecuteEmptyInsert`); its master pick #65105 changes `IcebergInsertExecutor`, which master no longer has. #69010 made `PluginDrivenInsertExecutor.requiresTransaction()` true for every connector overwrite, which covers #62344's case and the others. Both connectors already commit an overwrite that receives no data: Paimon commits `withOverwrite(staticPartition)` with no messages, and Iceberg overwrites the matching files with no new ones. **The problem, and what it cost.** No test checks any of this. `PluginDrivenInsertExecutorTest` has no case for `requiresTransaction()`, and the empty overwrites in the regression suites never put the sink on the empty relation, so they pass with or without #69010's change: | Existing case | Plan on master | Without #69010's `requiresTransaction` override | |---|---|---| | `test_iceberg_partition_overwrite_mode`, `static_optimized_empty` (partitioned table, `WHERE false`, strict DML off) | hash exchange for the partition routing, the writers run | clears the table anyway | | `test_paimon_write_transaction`, `txn_empty_overwrite_limit_zero` (append-only table, `LIMIT 0`) | `ExecutionAny` exchange, the writers run | clears the table anyway | On master the sink sits directly on the empty relation for a Paimon primary-key table without a fixed bucket count, which is Paimon's default and is written by a single writer, and for an unpartitioned Iceberg table when `enable_strict_consistency_dml` is false, the default in cloud mode. On a local cluster with an FE built without the override, that is, master before #69010: | Statement | Without the override | master | |---|---|---| | `INSERT OVERWRITE TABLE t SELECT 1, 1 LIMIT 0`, Paimon primary-key table without `bucket` | returns OK, both rows stay, no snapshot | table cleared, OVERWRITE snapshot | | `INSERT OVERWRITE TABLE t PARTITION (p = 'a') SELECT 1, 1 LIMIT 0`, the same kind of table, partitioned | returns OK, both partitions stay | only partition `a` cleared | | `INSERT OVERWRITE TABLE t SELECT * FROM t LIMIT 0`, unpartitioned Iceberg table, strict DML off | returns OK, all three rows stay | table cleared | So a change that brought the shortcut back, or a connector that returned early when it receives no data, would silently skip such overwrites again, and no test would notice. branch-4.2 still skips the first and the third (only an Iceberg static partition overwrite runs its transaction there); that is reported to its maintainers separately. **How this PR fixes it.** Tests only: - `PluginDrivenInsertExecutorTest`: an empty insert that overwrites the whole table or a static partition requires its transaction; an empty `INSERT` does not. - `PaimonConnectorTransactionTest`: an overwrite that receives no commit data replaces its static partition with nothing and keeps the other partition, and a whole-table overwrite clears the table, both with an OVERWRITE snapshot. The table is a real on-disk Paimon table written through the Paimon API. - `IcebergConnectorTransactionTest`: an Iceberg static partition overwrite with no data deletes only the file of the matching partition, ported from the branch-4.1 test of #62344. - `test_paimon_write_transaction`: `LIMIT 0` overwrites of a primary-key table without `bucket` and of a static partition of one. `EXPLAIN SHAPE PLAN` pins the sink on `PhysicalEmptyRelation`; the suite then checks the rows, the snapshots, and what Spark reads. The comment of the existing append-only `LIMIT 0` case said "A direct PhysicalEmptyRelation"; it now says that the table is written through an exchange. - `test_iceberg_insert_overwrite_with_empty_table`: a `LIMIT 0` overwrite of an unpartitioned table with strict DML consistency off, with its plan. **Results.** - FE UT: `PluginDrivenInsertExecutorTest` passes (`run-fe-ut.sh`, 11 cases). Removing the override turns the new overwrite case red; requiring a transaction for every empty insert turns the new INSERT case red. The new Paimon cases fail when the connector also returns early for an overwrite with no commit data, and the Iceberg case fails when a static partition overwrite with no data returns early. All 826 tests of fe-connector-paimon and all 1611 of fe-connector-iceberg pass. - Regression: both suites pass on a local cluster against the iceberg docker environment, and fail with the override removed (at `txn_dynamic_bucket_empty_overwrite` and `q4`). The regenerated `.out` files differ from master's only by the new blocks. Original author: @suxiaogang223 (the Iceberg static partition overwrite test, from #62344)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What problem does this PR solve?
Issue Number: None
Related PR: #62344
Problem Summary: Pick #62344 from branch-4.1 to master. The source PR fixes Iceberg static partition overwrite when the source query is optimized to an empty relation, and stabilizes the information_schema.rowsets timezone regression case.
git cherryshowed the three source commits were not present on origin/master, so this PR applies them to master. The only pick conflict was in test_information_schema_timezone.groovy; the resolution keeps the current master offset timezone coverage and applies the stable rowsets query helper from the source PR.Release note
Fix Iceberg static partition overwrite when the source query is empty.
Check List (For Author)