apache / apache/doris

[Bug] Two FE UT failures on current master

Open
#67,403 0 comments 0 reactions 0 assignees View on GitHub
Dominant language
Java
Stars
15.9k
Forks
3.9k
Avg merge
2d 23h
Merged PRs (30d)
520

Description

### Search before asking

- [X] I had searched in the [issues](https://github.com/apache/doris/issues) and found no similar issues.

### Version

master: `5fbe3600024` (Sep 1); failure 1 also reproduced on `d454100765c` (Aug 31 master)

### What's Wrong?

Two FE unit tests fail on current master itself. Both are test-vs-code contradictions introduced by upstream PRs landing in a conflicting order.

Found while running the full local FE unit test suite to validate #67310; unrelated to that PR (it only touches `MasterOpExecutor` / `FrontendServiceImpl` / thrift definitions, zero overlap with these modules).

**1. `IcebergWritePlanProviderTest.planMergePreservesExplicitlyEmptyReadAcrossConcurrentFirstAppend`**

```
org.opentest4j.AssertionFailedError: an explicitly empty read must leave RowDelta validation unbounded across the first append ==> expected: but was: <-1>
at org.apache.doris.connector.iceberg.IcebergWritePlanProviderTest
.planMergePreservesExplicitlyEmptyReadAcrossConcurrentFirstAppend(IcebergWritePlanProviderTest.java:1727)
```

- The test was added by #66345 (Aug 6) asserting `baseSnapshotId == null` for an explicitly pinned empty read.
- #66348 (Aug 14) deliberately changed the semantics: an explicitly pinned `-1` is kept as an OCC fence (`IcebergConnectorTransaction.applyBeginGuards`, comment *"An explicitly pinned -1 is the empty-table generation and must remain an OCC fence"*), and the `null`-arm was replaced by `Long.valueOf(pinnedReadSnapshot)`.
- The old assertion was not updated, so this test has been failing on master since Aug 14.

**2. `PropertyAnalyzerTest.testAnalyzePartitionInvertedIndexFileStorageFormat`**

```
org.opentest4j.AssertionFailedError: expected: but was:
at org.apache.doris.common.PropertyAnalyzerTest
.testAnalyzePartitionInvertedIndexFileStorageFormat(PropertyAnalyzerTest.java:412)
```

- The test was added by #66475 (Sep 1) and expects `partition.inverted_index_storage_format = "V1"` to be rejected with the partition-level message *"only supports V2, V3 and SNII"*.
- But #64522 (Aug 24) already made `analyzeInvertedIndexFileStorageFormat` throw *"Inverted index V1 is deprecated and no longer allowed for new index creation"* for any `v1` input — which fires **before** the partition-level `V1` check, making that check (and the expected message) unreachable dead code.

### What You Expected?

Both tests should pass on master.

### How to Reproduce?

1. `cd fe && mvn test -pl fe-connector/fe-connector-iceberg -am -Dtest=IcebergWritePlanProviderTest`
2. `cd fe && mvn test -pl fe-core -am -Dtest=PropertyAnalyzerTest`

Suggested fixes:

- Failure 1: update the assertion to `assertEquals(Long.valueOf(-1L), txn.getBaseSnapshotId())` (matching the intent documented in #66348).
- Failure 2: either accept either rejection message in the assertion, or have `analyzePartitionInvertedIndexFileStorageFormat` surface its own message for `V1` before delegating to `analyzeInvertedIndexFileStorageFormat`.

### Anything Else?

- JDK 17, Maven 3.9.16, single-fork surefire
- Failing test files verified identical to master (zero diff)

### Are you willing to submit PR?

- [X] Yes I am willing to submit a PR!

Contributor guide

Open the contributing guide

Research direction

Run the two Maven commands from the issue, then inspect IcebergWritePlanProviderTest.java at line 1727 and PropertyAnalyzerTest.java at line 412 alongside the referenced upstream behavior. Update the tests or surrounding assertion behavior to match the intended semantics, and confirm both targeted test classes pass.

Written by the indexing model from the issue text.

Assessment

Tech stack
java
Domain
databases, testing
Issue type
Bug
Difficulty
3/5
Estimated time
1-2 days
Activity status
Active
Clarity
Clearly specified
Newbie friendliness
78/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.