This is an automated email from the ASF dual-hosted git repository.
morningman pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/doris.git
The following commit(s) were added to refs/heads/master by this push:
new 168d0777833 [fix](build) Unbreak master: stale unity-skip entry (BE)
and dropped count probe (FE) (#66831)
168d0777833 is described below
commit 168d07778338761de5da5f0e97c36eee1e265b4b
Author: Mingyu Chen (Rayner) <[email protected]>
AuthorDate: Mon Aug 17 20:29:25 2026 +0800
[fix](build) Unbreak master: stale unity-skip entry (BE) and dropped count
probe (FE) (#66831)
### What
Two independent breakages that each make **current master fail to
build** — one in BE configure, one in FE compile.
They share a shape: a pair of PRs that never conflict textually, merge
cleanly, and only break once combined, so each PR's own pipeline was
green.
| | Breakage | Colliding PRs |
|---|---|---|
| BE | `cmake` configure aborts | #66052 moved a file, #66789 made a
dangling unity-skip entry fail loud |
| FE | `fe-connector-iceberg` does not compile | #66778 deleted
`getCountFromSnapshot()`, #66413 added a caller for it |
CI merges each PR into the latest master before building, so **every PR
pipeline that picks up current master is red** on one or both.
---
## 1. BE — stale `STORAGE_UNITY_SKIP` entry for a moved file
Remove the stale `STORAGE_UNITY_SKIP` entry (and its comment block) for
`compaction/collection_statistics.cpp`, which no longer exists.
### Why — master configure is currently broken
#66052 moved `storage/compaction/collection_statistics.{cpp,h}` to
`storage/index/inverted/similarity/` (rewritten), but left behind the
unity-skip entry that #66789 had added for the old path. The fail-loud
validation introduced by #66789 turns a dangling skip entry into a
configure-time error — which is exactly what it is designed to catch (a
skip list rotting after a file move), so BE configure on current master
fails immediately:
```
CMake Error at CMakeLists.txt:1002 (message):
unity skip entry does not exist (renamed or moved?):
.../be/src/storage/compaction/collection_statistics.cpp
```
#66826, #66824, #66819, #66820 were the first hits — same error on
multiple independent agents.
### Why deletion (not a path update) is correct
The old entry existed because the old `collection_statistics_test`
`#include`d the `.cpp` into a second TU (unity batching would then
produce a duplicate definition at link time). The rewritten file at the
new location is not `#include`d by any test (`grep -rn
'collection_statistics.cpp' be/test/` is empty on master), so the new
path needs no skip entry.
### Verification
- Full BE build from a clean tree at master + this change (clang20 /
macOS arm64, unity=ON, PCH=ON): configure passes the skip-list
validation and the build compiles. (The same tree without this change
fails configure with the error above.)
- Timeline note: #66052's last green CI round presumably predates
#66789's validation landing (2026-08-16), which is how the dangling
entry slipped through.
---
## 2. FE — the metadata-only COUNT(\*) probe calls a deleted method
#66778 replaced the snapshot-summary COUNT(\*) pushdown with a
manifest-derived count and deleted `getCountFromSnapshot()`.
`canServeMetadataOnlyCount()`, added by #66413, still calls it, so FE
compilation fails:
```
[ERROR] .../connector/iceberg/IcebergScanPlanProvider.java:[505,16] cannot
find symbol
[ERROR] symbol: method
getCountFromSnapshot(org.apache.iceberg.TableScan,org.apache.doris.connector.spi.ConnectorSession)
[ERROR] location: class
org.apache.doris.connector.iceberg.IcebergScanPlanProvider
```
### Why re-express the probe instead of restoring the method
Bringing `getCountFromSnapshot()` back would reintroduce precisely what
#66778 removed: a query result derived from optional, writer-provided
snapshot summary fields.
The probe is rebuilt in #66778's own terms instead. It reuses that PR's
delete gate and additionally requires the data manifests to carry
aggregate row counters, so the answer is proved from the manifest list
alone — `O(manifests)`, no data-file enumeration, which is what a probe
running before planning can afford.
Manifest lists that omit those aggregates now answer `false`, where
count planning still serves them through its bounded per-file fallback.
A capability probe should under-promise rather than over-promise; the
planner itself is untouched.
### Verification
- `mvn package` over the full FE reactor (74 modules): all green. This
also confirms no second semantic break is hiding behind the first — CI's
maven stops at `fe-connector-iceberg` and never reaches the rest.
- `IcebergScanPlanProviderTest`: 153 tests, 0 failures, including a new
case pinning that the probe follows the same delete gate as count
planning.
- FE checkstyle: 0 violations.
### Release note
None (both fixes only restore a buildable master; no user-visible
behavior change).
### Check List (For Author)
- Test
- [x] Regression test — not applicable: neither change alters query
behavior
- [x] Unit test: `IcebergScanPlanProviderTest` (153 tests, 0 failures)
- [x] Manual test: full BE configure/build and full FE reactor build
- Behavior changed: No
- Does this need documentation: No
---------
Co-authored-by: Claude Fable 5 <[email protected]>
---
be/src/storage/CMakeLists.txt | 7 +----
.../connector/iceberg/IcebergScanPlanProvider.java | 36 ++++++++++++++++++++--
.../iceberg/IcebergScanPlanProviderTest.java | 20 +++++++++++-
3 files changed, 54 insertions(+), 9 deletions(-)
diff --git a/be/src/storage/CMakeLists.txt b/be/src/storage/CMakeLists.txt
index 21e82183f58..306b4815f20 100644
--- a/be/src/storage/CMakeLists.txt
+++ b/be/src/storage/CMakeLists.txt
@@ -55,10 +55,6 @@ pch_reuse(Storage)
# FORMAT_*_ADD_JSON_NODE, RETURN_IF_ERROR_) must not leak into unity siblings
# - the three heaviest template-instantiation TUs (predicate creators) which
# would dominate any batch they join
-# - compaction/collection_statistics.cpp: its test compiles it a second time
-# by #including the .cpp; the test object must shadow a never-pulled archive
-# member, but a unity batch is pulled in for its siblings and the linker
-# sees a duplicate definition
set(STORAGE_UNITY_SKIP
${CMAKE_CURRENT_SOURCE_DIR}/index/inverted/inverted_index_compound_reader.cpp
${CMAKE_CURRENT_SOURCE_DIR}/index/inverted/inverted_index_fs_directory.cpp
@@ -69,8 +65,7 @@ set(STORAGE_UNITY_SKIP
${CMAKE_CURRENT_SOURCE_DIR}/task/engine_clone_task.cpp
${CMAKE_CURRENT_SOURCE_DIR}/predicate/predicate_creator_comparison.cpp
${CMAKE_CURRENT_SOURCE_DIR}/predicate/predicate_creator_in_list_in.cpp
- ${CMAKE_CURRENT_SOURCE_DIR}/predicate/predicate_creator_in_list_not_in.cpp
- ${CMAKE_CURRENT_SOURCE_DIR}/compaction/collection_statistics.cpp)
+ ${CMAKE_CURRENT_SOURCE_DIR}/predicate/predicate_creator_in_list_not_in.cpp)
if (ENABLE_VARIANT_NESTED_GROUP)
# Out-of-tree module sources swapped into this target: unity hygiene
# unaudited, keep them individual.
diff --git
a/fe/fe-connector/fe-connector-iceberg/src/main/java/org/apache/doris/connector/iceberg/IcebergScanPlanProvider.java
b/fe/fe-connector/fe-connector-iceberg/src/main/java/org/apache/doris/connector/iceberg/IcebergScanPlanProvider.java
index bd2e90bd646..24c2a5ec0d1 100644
---
a/fe/fe-connector/fe-connector-iceberg/src/main/java/org/apache/doris/connector/iceberg/IcebergScanPlanProvider.java
+++
b/fe/fe-connector/fe-connector-iceberg/src/main/java/org/apache/doris/connector/iceberg/IcebergScanPlanProvider.java
@@ -497,12 +497,12 @@ public class IcebergScanPlanProvider implements
ConnectorScanPlanProvider {
Optional<ConnectorExpression> filter) {
IcebergTableHandle iceHandle = (IcebergTableHandle) handle;
if (iceHandle.isSystemTable() || filter.isPresent()) {
- // Snapshot summaries describe the whole table and cannot prove a
filtered row count.
+ // A metadata count describes the whole table and cannot prove a
filtered row count.
return false;
}
Table table = resolveTable(session, iceHandle);
TableScan scan = buildScan(table, iceHandle, filter, session);
- return getCountFromSnapshot(scan, session) >= 0;
+ return canProveCountFromManifests(table, scan, session);
}
/**
@@ -1312,6 +1312,38 @@ public class IcebergScanPlanProvider implements
ConnectorScanPlanProvider {
netPositionDeletes ? positionDeleteRows.getAsLong() : 0);
}
+ /**
+ * The capability probe behind {@link #canServeMetadataOnlyCount}: the
delete gate of
+ * {@link #planCountPushdown} plus the requirement that the data manifests
really carry aggregate row
+ * counters. Reading only the manifest list keeps this O(manifests) with
no data-file enumeration, which is
+ * what a pre-planning probe can afford; the price is answering {@code
false} for the older manifest lists
+ * that {@code planCountPushdown} still serves through its bounded
per-file fallback. Never derives the
+ * count from snapshot summary fields — those are writer-provided hints,
not a query result.
+ */
+ private static boolean canProveCountFromManifests(Table table, TableScan
scan, ConnectorSession session) {
+ Snapshot snapshot = scan.snapshot();
+ if (snapshot == null) {
+ // No snapshot (empty table, or a pinned empty snapshot) is an
exact count of 0 without any read.
+ return true;
+ }
+ boolean netPositionDeletes = sessionBool(session,
IGNORE_ICEBERG_DANGLING_DELETE, false);
+ ManifestDeleteState deleteState =
manifestDeleteState(snapshot.deleteManifests(table.io()));
+ if (deleteState == ManifestDeleteState.PRESENT && !netPositionDeletes)
{
+ return false;
+ }
+ OptionalLong positionDeleteRows = deleteState ==
ManifestDeleteState.NONE
+ ? OptionalLong.of(0)
+ : livePositionDeleteRowCount(table, snapshot);
+ if (!positionDeleteRows.isPresent()
+ || (!netPositionDeletes && positionDeleteRows.getAsLong() !=
0)) {
+ return false;
+ }
+ OptionalLong manifestCount =
liveRowCountFromManifests(snapshot.dataManifests(table.io()));
+ return manifestCount.isPresent()
+ && subtractPositionDeleteRows(manifestCount.getAsLong(),
+ netPositionDeletes ? positionDeleteRows.getAsLong() :
0).isPresent();
+ }
+
private Optional<List<ConnectorScanRange>> planManifestCountRange(Table
table, TableScan scan, long exactCount,
int formatVersion, boolean partitioned, List<String>
orderedPartitionKeys, ZoneId zone,
UnaryOperator<String> uriNormalizer, ConnectorSession session,
Optional<ConnectorExpression> filter,
diff --git
a/fe/fe-connector/fe-connector-iceberg/src/test/java/org/apache/doris/connector/iceberg/IcebergScanPlanProviderTest.java
b/fe/fe-connector/fe-connector-iceberg/src/test/java/org/apache/doris/connector/iceberg/IcebergScanPlanProviderTest.java
index 73c0abd20a9..abbcb46a8e0 100644
---
a/fe/fe-connector/fe-connector-iceberg/src/test/java/org/apache/doris/connector/iceberg/IcebergScanPlanProviderTest.java
+++
b/fe/fe-connector/fe-connector-iceberg/src/test/java/org/apache/doris/connector/iceberg/IcebergScanPlanProviderTest.java
@@ -1871,7 +1871,7 @@ public class IcebergScanPlanProviderTest {
}
@Test
- public void metadataOnlyCountCapabilityUsesSnapshotSummary() {
+ public void metadataOnlyCountCapabilityUsesManifestAggregates() {
Table table = createTable("t1", SCHEMA, PartitionSpec.unpartitioned());
table.newAppend().appendFile(dataFile(
table.spec(), "s3://b/db/t1/f1.parquet", 1000, null,
null)).commit();
@@ -1887,6 +1887,24 @@ public class IcebergScanPlanProviderTest {
"db1", "t1", "snapshots", -1L, null, -1L),
Optional.empty()));
}
+ @Test
+ public void metadataOnlyCountCapabilityFollowsTheDeleteGate() {
+ // The capability must follow the same delete gate as count planning:
live deletes leave the row count
+ // unprovable from manifests alone. MUTATION: reporting the capability
from data manifests only -> red.
+ Table table = createTable("t1", SCHEMA, PartitionSpec.unpartitioned(),
+ Collections.singletonMap(TableProperties.FORMAT_VERSION, "2"));
+ table.newAppend().appendFile(dataFile(
+ table.spec(), "s3://b/db/t1/f1.parquet", 1000, null,
null)).commit();
+ table.newRowDelta().addDeletes(
+ positionDeleteFile("s3://b/db/t1/pos.parquet",
FileFormat.PARQUET, null, null)).commit();
+ IcebergScanPlanProvider provider = new IcebergScanPlanProvider(
+ IcebergCatalogProperties.of(Collections.emptyMap()),
opsReturning(table));
+ ConnectorSession session = new FakeScanSession("UTC",
Collections.emptyMap());
+
+ Assertions.assertFalse(provider.canServeMetadataOnlyCount(
+ session, new IcebergTableHandle("db1", "t1"),
Optional.empty()));
+ }
+
@Test
public void getScanNodePropertiesUnderPinEmitsFullPinnedSchemaDict()
throws Exception {
// T07 Option A: under a time-travel pin the field-id dict is built
from the FULL pinned schema (covering
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
