Skip to content

branch-4.1: [fix](iceberg) execute empty static overwrite and stabilize info_schema timezone test - #62344

Merged
yiguolei merged 3 commits into
apache:branch-4.1from
suxiaogang223:codex/fix-empty-static-overwrite
Apr 11, 2026
Merged

yiguolei merged 3 commits into
apache:branch-4.1from
suxiaogang223:codex/fix-empty-static-overwrite

Conversation

@suxiaogang223

Copy link
Copy Markdown
Member

What problem does this PR solve?

This PR fixes two regression issues on branch-4.1.

  1. Iceberg static partition overwrite was incorrectly treated as a no-op when the source query was optimized to an empty relation.
  2. test_information_schema_timezone was flaky around information_schema.rowsets.

Solution

  • Allow empty-source execution for Iceberg static partition overwrite so the transaction still reaches overwriteByRowFilter().
  • Add a FE unit test covering empty-source static partition overwrite deleting only the matching partition.
  • Stabilize the information_schema.rowsets regression case by waiting for a rowset result, ordering deterministically, and guarding nullable timestamps.

Test Result

  • DISABLE_BUILD_UI=ON DISABLE_BUILD_HIVE_UDF=ON DISABLE_BE_JAVA_EXTENSIONS=ON ./build.sh --fe

@suxiaogang223
suxiaogang223 requested a review from yiguolei as a code owner April 10, 2026 09:48
@hello-stephen

Copy link
Copy Markdown
Contributor

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:

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

@suxiaogang223

Copy link
Copy Markdown
Member Author

run buildall

@suxiaogang223

Copy link
Copy Markdown
Member Author

run buildall

@hello-stephen

Copy link
Copy Markdown
Contributor

FE UT Coverage Report

Increment line coverage 7.14% (2/28) 🎉
Increment coverage report
Complete coverage report

@yiguolei

Copy link
Copy Markdown
Contributor

skip buildall

@github-actions github-actions Bot added the approved Indicates a PR has been approved by one committer. label Apr 11, 2026
@github-actions

Copy link
Copy Markdown
Contributor

PR approved by at least one committer and no changes requested.

@github-actions

Copy link
Copy Markdown
Contributor

PR approved by anyone and no changes requested.

@yiguolei
yiguolei merged commit 9c7db5b into apache:branch-4.1 Apr 11, 2026
28 of 32 checks passed
@suxiaogang223
suxiaogang223 deleted the codex/fix-empty-static-overwrite branch July 2, 2026 08:36
morningman added a commit to morningman/doris that referenced this pull request Oct 21, 2026
…nner proves empty

### What problem does this PR solve?

Issue Number: None

Related PR: apache#69010, apache#62344

Problem Summary: When the planner folds the source of an INSERT OVERWRITE to an
empty relation (LIMIT 0, WHERE false) and no exchange separates the sink from it,
the insert is an empty insert. Before apache#69010 an empty insert skipped its
transaction, so such an overwrite returned OK and the target kept all its rows.
branch-4.1 apache#62344 runs that transaction only for an Iceberg static partition
overwrite; master apache#69010 runs it for every connector overwrite
(PluginDrivenInsertExecutor.requiresTransaction). Master behaves correctly, but
no test covers it:

- PluginDrivenInsertExecutorTest has no case for requiresTransaction.
- The planner-proven empty overwrite in test_iceberg_partition_overwrite_mode
  writes a partitioned table, which master routes through a hash exchange, so
  the writers run and the case passes without the apache#69010 override.
- The LIMIT 0 overwrite in test_paimon_write_transaction writes an append-only
  table, which goes through an exchange as well.

The sink sits directly on the empty relation for a Paimon primary-key table
without a fixed bucket count (Paimon's default, 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 the apache#69010 override
removed, a LIMIT 0 overwrite of such a Paimon table, of a static partition of
one, and of such an Iceberg table returned OK and left every row in place. On
master all three clear their target.

This adds:
- PluginDrivenInsertExecutorTest: an empty overwrite, of the whole table or of a
  static partition, requires its transaction; an empty INSERT does not.
- PaimonConnectorTransactionTest: an overwrite that receives no commit data
  replaces its static partition, or the whole table, with nothing.
- IcebergConnectorTransactionTest: the same for an Iceberg static partition,
  ported from the branch-4.1 test of apache#62344.
- test_paimon_write_transaction: LIMIT 0 overwrites of a dynamic-bucket
  primary-key table and of a static partition of one, with the plan that puts
  the sink on the empty relation. The comment of the append-only case now says
  that it goes through an exchange.
- test_iceberg_insert_overwrite_with_empty_table: a LIMIT 0 overwrite of an
  unpartitioned table with strict DML consistency off.

### Release note

None

### Check List (For Author)

- Test: Unit Test, Regression test
    - PluginDrivenInsertExecutorTest (run-fe-ut.sh), PaimonConnectorTransactionTest
      and IcebergConnectorTransactionTest pass; each new case fails on a mutation
      of the code it covers.
    - test_paimon_write_transaction and test_iceberg_insert_overwrite_with_empty_table
      pass on a local cluster against the iceberg docker environment, and fail
      with the apache#69010 override removed.
- Behavior changed: No
- Does this need documentation: No

Co-authored-by: Socrates <[email protected]>
Co-Authored-By: Claude Opus 5.5 (1M context) <[email protected]>
morningman added a commit to morningman/doris that referenced this pull request Oct 23, 2026
…nner proves empty

### What problem does this PR solve?

Issue Number: None

Related PR: apache#69010, apache#62344

Problem Summary: When the planner folds the source of an INSERT OVERWRITE to an
empty relation (LIMIT 0, WHERE false) and no exchange separates the sink from it,
the insert is an empty insert. Before apache#69010 an empty insert skipped its
transaction, so such an overwrite returned OK and the target kept all its rows.
branch-4.1 apache#62344 runs that transaction only for an Iceberg static partition
overwrite; master apache#69010 runs it for every connector overwrite
(PluginDrivenInsertExecutor.requiresTransaction). Master behaves correctly, but
no test covers it:

- PluginDrivenInsertExecutorTest has no case for requiresTransaction.
- The planner-proven empty overwrite in test_iceberg_partition_overwrite_mode
  writes a partitioned table, which master routes through a hash exchange, so
  the writers run and the case passes without the apache#69010 override.
- The LIMIT 0 overwrite in test_paimon_write_transaction writes an append-only
  table, which goes through an exchange as well.

The sink sits directly on the empty relation for a Paimon primary-key table
without a fixed bucket count (Paimon's default, 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 the apache#69010 override
removed, a LIMIT 0 overwrite of such a Paimon table, of a static partition of
one, and of such an Iceberg table returned OK and left every row in place. On
master all three clear their target.

This adds:
- PluginDrivenInsertExecutorTest: an empty overwrite, of the whole table or of a
  static partition, requires its transaction; an empty INSERT does not.
- PaimonConnectorTransactionTest: an overwrite that receives no commit data
  replaces its static partition, or the whole table, with nothing.
- IcebergConnectorTransactionTest: the same for an Iceberg static partition,
  ported from the branch-4.1 test of apache#62344.
- test_paimon_write_transaction: LIMIT 0 overwrites of a dynamic-bucket
  primary-key table and of a static partition of one, with the plan that puts
  the sink on the empty relation. The comment of the append-only case now says
  that it goes through an exchange.
- test_iceberg_insert_overwrite_with_empty_table: a LIMIT 0 overwrite of an
  unpartitioned table with strict DML consistency off.

### Release note

None

### Check List (For Author)

- Test: Unit Test, Regression test
    - PluginDrivenInsertExecutorTest (run-fe-ut.sh), PaimonConnectorTransactionTest
      and IcebergConnectorTransactionTest pass; each new case fails on a mutation
      of the code it covers.
    - test_paimon_write_transaction and test_iceberg_insert_overwrite_with_empty_table
      pass on a local cluster against the iceberg docker environment, and fail
      with the apache#69010 override removed.
- Behavior changed: No
- Does this need documentation: No

Co-authored-by: Socrates <[email protected]>
Co-Authored-By: Claude Opus 5.5 (1M context) <[email protected]>
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)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approved Indicates a PR has been approved by one committer. dev/4.1.0-merged reviewed

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants