ClickHouse / ClickHouse/ClickHouse
UniqueKeyProbeTest.NewestLiveWinsOverOlder aborts unit_tests_dbms in Debug builds (single-live-part invariant vs. the test's own setup)
- Dominant language
- C++
- Stars
- 49.9k
- Forks
- 9k
- Avg merge
- 21h 32m
- Merged PRs (30d)
- 515
Description
`UniqueKeyProbeTest.NewestLiveWinsOverOlder` aborts the whole `unit_tests_dbms` binary in Debug builds, because the test constructs exactly the state that the debug-only single-live-part invariant forbids.
## Reproduction
Debug build (`clang-21`, `CMAKE_BUILD_TYPE=Debug`), current `master` (`f70042f015b`):
```
$ build/src/unit_tests_dbms --gtest_filter="UniqueKeyProbeTest.NewestLiveWinsOverOlder"
[ RUN ] UniqueKeyProbeTest.NewestLiveWinsOverOlder
Logical error: 'UNIQUE KEY probe invariant violated: key at batch row 0 is live in more than one part (partition 'p0')'.
...
5. src/Storages/MergeTree/UniqueKey/UniqueKeyProbeSimple.cpp:91: DB::UniqueKeyProbeSimple::probeBatch(DB::Block const&, String const&)
6. src/Storages/MergeTree/UniqueKey/tests/gtest_unique_key_probe.cpp:175: UniqueKeyProbeTest::probeKey(DB::IUniqueKeyProbe&, unsigned long)
7. src/Storages/MergeTree/UniqueKey/tests/gtest_unique_key_probe.cpp:215: UniqueKeyProbeTest_NewestLiveWinsOverOlder_Test::TestBody()
Aborted (core dumped) # exit code 134
```
## Cause
`UniqueKeyProbeSimple.cpp` gates the early-skip on build type:
```cpp
/// Debug builds validate the "a live key lives in <= 1 part" invariant by probing
/// every part for every key; release builds early-skip a key once it resolves live.
#ifndef NDEBUG
static constexpr bool probe_validates_single_live_part = true;
#else
static constexpr bool probe_validates_single_live_part = false;
#endif
```
and then, in `probeBatch`:
```cpp
/// See `probe_validates_single_live_part`: release skips, debug does not.
if (resolved[i] && !probe_validates_single_live_part)
continue;
...
if (!target->isRowDead(*row_opt))
{
if (resolved[i])
throw Exception(ErrorCodes::LOGICAL_ERROR,
"UNIQUE KEY probe invariant violated: key at batch row {} is live in more "
"than one part (partition '{}')", i, partition_id);
```
The test sets up key `5` as **live in two parts**:
```cpp
TEST_F(UniqueKeyProbeTest, NewestLiveWinsOverOlder)
{
auto newest = makeTarget({{5, 9}});
auto older = makeTarget({{5, 1}});
...
auto probe = probeOver({newest, older}); /// newest-first
auto r = probeKey(probe, 5);
EXPECT_EQ(r.outcome, ProbeOutcome::FOUND_LIVE);
EXPECT_EQ(r.row_number, 9u);
}
```
In Release the early-skip fires after the first live hit, so the second live hit is never observed and the test passes. In Debug the skip is disabled precisely so that a second live hit is detected — and this test guarantees one. The test and the invariant contradict each other by construction.
## Which side is wrong is a question for the owners
Two readings, and I do not know which is intended:
1. **The invariant is right** — a live key really must exist in at most one active part, so this test constructs impossible data. It should then assert the throw, or be reworked to express "newest wins" without two live parts.
2. **The test is right** — "walk newest-first, first live hit wins" is legitimate while two parts transiently hold the same key, in which case the debug validation is too strong and should not treat a second live hit as a logical error.
The neighbouring `SkipNewerDeadFindOlderLive` covers newest-first resolution with the newer row bitmap-dead, which suggests reading 1, but that is a guess.
## Why it is worth fixing promptly
The abort takes down the entire `unit_tests_dbms` process, so **every test that would have run after it is silently skipped**. In my run that hid the remainder of the suite: gtest prints no failure summary, the process dies with signal 6, and a wrapper that inspects only the last command's status can report success. Excluding this one test brings the suite to 22712 passed, 0 failed.
## Origin
The `probe_validates_single_live_part` gate was introduced by `de277e15ae0` (2026-07-16, *"UK PR-9b: force unmasked SST rebuild read; comment/dead-code cleanup"*). The test predates it — it moved to its current location in `1c8b51dca8b` (2026-06-05) — so the two most likely diverged when the debug validation was added to code the test had been passing against.
I found this while running the Debug unit suite for unrelated work on `Nullable(Array)` support. I confirmed it is not caused by my changes by reverting them and reproducing the abort unchanged.
Related: https://github.com/ClickHouse/ClickHouse/issues/115394
Contributor guide
Assessment
This issue has not been assessed yet.