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 32a2651f66b [fix](iceberg) Fix NPE in COUNT(*) pushdown when snapshot
summary omits total-* counters (#64648)
32a2651f66b is described below
commit 32a2651f66b9f0f341c90e95f8ed304b34927111
Author: Raghvendra Singh <[email protected]>
AuthorDate: Wed Jul 1 08:14:59 2026 +0530
[fix](iceberg) Fix NPE in COUNT(*) pushdown when snapshot summary omits
total-* counters (#64648)
## Proposed changes
`IcebergScanNode.getCountFromSnapshot()` reads `total-equality-deletes`,
`total-position-deletes` and `total-records` from the Iceberg snapshot
summary
and calls `.equals()` / `Long.parseLong()` directly on the `Map.get()`
results.
An Iceberg snapshot summary is **not guaranteed** to carry these
`total-*`
counters — snapshots written by compaction/replace (and some writers)
may omit
them. When a counter is absent, `SELECT COUNT(*)` throws:
```
java.lang.NullPointerException: Cannot invoke "String.equals(Object)"
because
the return value of "java.util.Map.get(Object)" is null
at
org.apache.doris.datasource.iceberg.source.IcebergScanNode.getCountFromSnapshot(IcebergScanNode.java:1154)
at
org.apache.doris.datasource.iceberg.source.IcebergScanNode.isBatchMode(...)
at
org.apache.doris.datasource.FileQueryScanNode.createScanRangeLocations(...)
```
`SELECT *` on the same table works (it scans); only `COUNT(*)` fails (it
takes
the metadata-count shortcut). Reproducible on current master.
### Fix
Extract the summary parsing into a pure static
`getCountFromSummary(Map<String, String> summary, boolean
ignoreDanglingDelete)`
that null-checks the counters and **falls back to a normal scan**
(`return -1`,
the method's existing "cannot push down count" signal) when any required
counter is absent. Behaviour is otherwise unchanged.
> A similar unguarded access exists in `IcebergUtils`
> (`Long.parseLong(summary.get(TOTAL_RECORDS)) -
Long.parseLong(summary.get(TOTAL_POSITION_DELETES))`).
> Happy to guard it in this PR or a follow-up — let me know your
preference.
## Release note
Fix a NullPointerException on `SELECT COUNT(*)` over an Iceberg table
whose
latest snapshot summary omits the `total-*` counters (e.g. snapshots
produced
by compaction/replace).
## Check List
- [x] New feature / bug fix has unit test
(`IcebergCountPushDownTest`: missing-counter, no-delete, equality-delete
and
position-delete cases)
- [x] Behaviour-preserving for complete summaries; only adds a null-safe
fall-back to scan
---------
Co-authored-by: Claude Opus 4.8 (1M context) <[email protected]>
Co-authored-by: morningman <[email protected]>
---
be/src/format/table/iceberg_reader_mixin.h | 8 +-
.../doris/datasource/iceberg/IcebergUtils.java | 35 ++++++++-
.../datasource/iceberg/source/IcebergScanNode.java | 19 +----
.../iceberg/source/IcebergCountPushDownTest.java | 86 ++++++++++++++++++++++
4 files changed, 127 insertions(+), 21 deletions(-)
diff --git a/be/src/format/table/iceberg_reader_mixin.h
b/be/src/format/table/iceberg_reader_mixin.h
index bd049342195..8ca30881dac 100644
--- a/be/src/format/table/iceberg_reader_mixin.h
+++ b/be/src/format/table/iceberg_reader_mixin.h
@@ -365,10 +365,14 @@ protected:
template <typename BaseReader>
Status IcebergReaderMixin<BaseReader>::_init_row_filters() {
- // COUNT(*) short-circuit
+ // COUNT(*) short-circuit. A table-level row count of 0 (e.g. an
all-deleted table read with
+ // ignore_iceberg_dangling_delete, where total-records ==
total-position-deletes) is still a
+ // valid pushed-down count, so accept >= 0 -- matching FileScanner and the
Paimon readers. FE
+ // sends -1 when there is no table-level count; using > 0 here would drop
a genuine 0 into the
+ // delete-applying path below and never produce the intended
CountReader(0).
if (this->_push_down_agg_type == TPushAggOp::type::COUNT &&
this->get_scan_range().table_format_params.__isset.table_level_row_count &&
- this->get_scan_range().table_format_params.table_level_row_count > 0) {
+ this->get_scan_range().table_format_params.table_level_row_count >= 0)
{
return Status::OK();
}
diff --git
a/fe/fe-core/src/main/java/org/apache/doris/datasource/iceberg/IcebergUtils.java
b/fe/fe-core/src/main/java/org/apache/doris/datasource/iceberg/IcebergUtils.java
index 92c6171b669..777250d7020 100644
---
a/fe/fe-core/src/main/java/org/apache/doris/datasource/iceberg/IcebergUtils.java
+++
b/fe/fe-core/src/main/java/org/apache/doris/datasource/iceberg/IcebergUtils.java
@@ -216,6 +216,34 @@ public class IcebergUtils {
}
}
+ /**
+ * Decide whether a row count can be read from an Iceberg snapshot summary.
+ * Returns {@link TableIf#UNKNOWN_ROW_COUNT} when required counters are
absent
+ * or when delete semantics make the summary count unsafe to use.
+ */
+ @VisibleForTesting
+ public static long getCountFromSummary(Map<String, String> summary,
boolean ignoreDanglingDelete) {
+ String equalityDeletes = summary.get(TOTAL_EQUALITY_DELETES);
+ String positionDeletes = summary.get(TOTAL_POSITION_DELETES);
+ String totalRecords = summary.get(TOTAL_RECORDS);
+ if (equalityDeletes == null || positionDeletes == null || totalRecords
== null) {
+ return TableIf.UNKNOWN_ROW_COUNT;
+ }
+ if (!equalityDeletes.equals("0")) {
+ return TableIf.UNKNOWN_ROW_COUNT;
+ }
+
+ long deleteCount = Long.parseLong(positionDeletes);
+ if (deleteCount == 0) {
+ return Long.parseLong(totalRecords);
+ }
+ if (ignoreDanglingDelete) {
+ return Long.parseLong(totalRecords) - deleteCount;
+ } else {
+ return TableIf.UNKNOWN_ROW_COUNT;
+ }
+ }
+
public static Expression convertToIcebergExpr(Expr expr, Schema schema) {
if (expr == null) {
return null;
@@ -1175,7 +1203,12 @@ public class IcebergUtils {
return TableIf.UNKNOWN_ROW_COUNT;
}
Map<String, String> summary = snapshot.summary();
- long rows = Long.parseLong(summary.get(TOTAL_RECORDS)) -
Long.parseLong(summary.get(TOTAL_POSITION_DELETES));
+ long rows = getCountFromSummary(summary, true);
+ if (rows == TableIf.UNKNOWN_ROW_COUNT) {
+ LOG.info("Iceberg table {}.{}.{} row count in summary is unknown,
return -1.",
+ tbl.getCatalog().getName(), tbl.getDbName(),
tbl.getName());
+ return TableIf.UNKNOWN_ROW_COUNT;
+ }
LOG.info("Iceberg table {}.{}.{} row count in summary is {}",
tbl.getCatalog().getName(), tbl.getDbName(), tbl.getName(),
rows);
return rows;
diff --git
a/fe/fe-core/src/main/java/org/apache/doris/datasource/iceberg/source/IcebergScanNode.java
b/fe/fe-core/src/main/java/org/apache/doris/datasource/iceberg/source/IcebergScanNode.java
index 2a359e509d0..2769a1ddb8e 100644
---
a/fe/fe-core/src/main/java/org/apache/doris/datasource/iceberg/source/IcebergScanNode.java
+++
b/fe/fe-core/src/main/java/org/apache/doris/datasource/iceberg/source/IcebergScanNode.java
@@ -1150,24 +1150,7 @@ public class IcebergScanNode extends FileQueryScanNode {
return 0;
}
- Map<String, String> summary = snapshot.summary();
- if (!summary.get(IcebergUtils.TOTAL_EQUALITY_DELETES).equals("0")) {
- // has equality delete files, can not push down count
- return -1;
- }
-
- long deleteCount =
Long.parseLong(summary.get(IcebergUtils.TOTAL_POSITION_DELETES));
- if (deleteCount == 0) {
- // no delete files, can push down count directly
- return Long.parseLong(summary.get(IcebergUtils.TOTAL_RECORDS));
- }
- if (sessionVariable.ignoreIcebergDanglingDelete) {
- // has position delete files, if we ignore dangling delete, can
push down count
- return Long.parseLong(summary.get(IcebergUtils.TOTAL_RECORDS)) -
deleteCount;
- } else {
- // otherwise, can not push down count
- return -1;
- }
+ return IcebergUtils.getCountFromSummary(snapshot.summary(),
sessionVariable.ignoreIcebergDanglingDelete);
}
@Override
diff --git
a/fe/fe-core/src/test/java/org/apache/doris/datasource/iceberg/source/IcebergCountPushDownTest.java
b/fe/fe-core/src/test/java/org/apache/doris/datasource/iceberg/source/IcebergCountPushDownTest.java
new file mode 100644
index 00000000000..19ad6bece44
--- /dev/null
+++
b/fe/fe-core/src/test/java/org/apache/doris/datasource/iceberg/source/IcebergCountPushDownTest.java
@@ -0,0 +1,86 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+package org.apache.doris.datasource.iceberg.source;
+
+import org.apache.doris.datasource.iceberg.IcebergUtils;
+
+import com.google.common.collect.ImmutableMap;
+import org.junit.jupiter.api.Assertions;
+import org.junit.jupiter.api.Test;
+
+import java.util.Collections;
+import java.util.Map;
+
+public class IcebergCountPushDownTest {
+
+ private static Map<String, String> summary(String equalityDeletes, String
positionDeletes, String totalRecords) {
+ ImmutableMap.Builder<String, String> builder = ImmutableMap.builder();
+ if (equalityDeletes != null) {
+ builder.put(IcebergUtils.TOTAL_EQUALITY_DELETES, equalityDeletes);
+ }
+ if (positionDeletes != null) {
+ builder.put(IcebergUtils.TOTAL_POSITION_DELETES, positionDeletes);
+ }
+ if (totalRecords != null) {
+ builder.put(IcebergUtils.TOTAL_RECORDS, totalRecords);
+ }
+ return builder.build();
+ }
+
+ @Test
+ public void testMissingCounterFallsBackToScan() {
+ // Snapshots written by compaction/replace (and some writers) may omit
+ // total-* counters. The pushdown previously NPE'd on the missing key;
+ // it must now fall back to a normal scan (return -1).
+ Assertions.assertEquals(-1L,
IcebergUtils.getCountFromSummary(summary(null, "0", "100"), false));
+ Assertions.assertEquals(-1L,
IcebergUtils.getCountFromSummary(summary("0", null, "100"), false));
+ Assertions.assertEquals(-1L,
IcebergUtils.getCountFromSummary(summary("0", "0", null), false));
+ Assertions.assertEquals(-1L,
IcebergUtils.getCountFromSummary(Collections.emptyMap(), false));
+ }
+
+ @Test
+ public void testUtilityMissingCounterReturnsUnknownCount() {
+ Assertions.assertEquals(-1L,
IcebergUtils.getCountFromSummary(summary("0", null, "100"), true));
+ }
+
+ @Test
+ public void testNoDeletesPushesDownTotalRecords() {
+ Assertions.assertEquals(100L,
IcebergUtils.getCountFromSummary(summary("0", "0", "100"), false));
+ }
+
+ @Test
+ public void testEqualityDeletesCannotPushDown() {
+ Assertions.assertEquals(-1L,
IcebergUtils.getCountFromSummary(summary("3", "0", "100"), false));
+ }
+
+ @Test
+ public void testPositionDeletesRespectIgnoreDangling() {
+ // ignoreDanglingDelete = true -> total-records minus position-deletes
+ Assertions.assertEquals(90L,
IcebergUtils.getCountFromSummary(summary("0", "10", "100"), true));
+ // ignoreDanglingDelete = false -> cannot push down (fall back to scan)
+ Assertions.assertEquals(-1L,
IcebergUtils.getCountFromSummary(summary("0", "10", "100"), false));
+ }
+
+ @Test
+ public void testZeroCountWithPositionDeletesIsPushedDown() {
+ // total-records == position-deletes -> count is 0. With
ignore_iceberg_dangling_delete this
+ // is a valid pushed-down count; FE returns 0 and BE honors it via
CountReader(0) (the BE
+ // table-level guard accepts table_level_row_count >= 0). It must NOT
fall back to -1.
+ Assertions.assertEquals(0L,
IcebergUtils.getCountFromSummary(summary("0", "100", "100"), true));
+ }
+}
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
