Skip to content

Fix NPE of approx_most_frequent and approx_count_distinct on empty input #19168

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
JackieTien97 merged 2 commits into from
Nov 7, 2026
+145 −25
Merged

Fix NPE of approx_most_frequent and approx_count_distinct on empty input #19168

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
9cd8ee1
Fix NPE of approx_most_frequent and approx_count_distinct on empty input
alpass163 Nov 6, 2026
38adbe7
fix BlobGroupedApproxMostFrequentAccumulator
alpass163 Nov 6, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
90 changes: 80 additions & 10 deletions ...st/src/test/java/org/apache/iotdb/relational/it/query/recent/IoTDBTableAggregationIT.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,7 @@ public class IoTDBTableAggregationIT {
"CREATE DATABASE " + DATABASE_NAME,
"USE " + DATABASE_NAME,
"CREATE TABLE table1(province STRING TAG, city STRING TAG, region STRING TAG, device_id STRING TAG, color STRING ATTRIBUTE, type STRING ATTRIBUTE, s1 INT32 FIELD, s2 INT64 FIELD, s3 FLOAT FIELD, s4 DOUBLE FIELD, s5 BOOLEAN FIELD, s6 TEXT FIELD, s7 STRING FIELD, s8 BLOB FIELD, s9 TIMESTAMP FIELD, s10 DATE FIELD)",
"CREATE TABLE empty_table(device_id STRING TAG, s1 INT32 FIELD, s2 INT64 FIELD, s6 TEXT FIELD, s8 BLOB FIELD)",
"INSERT INTO table1(time,province,city,region,device_id,color,type,s1,s3,s6,s8,s9) values (2024-09-24T06:15:30.000+00:00,'shanghai','shanghai','huangpu','d01','red','A',30,30.0,'shanghai_huangpu_red_A_d01_30', X'cafebabe30',2024-09-24T06:15:30.000+00:00)",
"INSERT INTO table1(time,province,city,region,device_id,color,type,s2,s3,s4,s6,s7,s9,s10) values (2024-09-24T06:15:35.000+00:00,'shanghai','shanghai','huangpu','d01','red','A',35000,35.0,35.0,'shanghai_huangpu_red_A_d01_35','shanghai_huangpu_red_A_d01_35',2024-09-24T06:15:35.000+00:00,'2024-09-24')",
"INSERT INTO table1(time,province,city,region,device_id,color,type,s1,s3,s5,s7,s9) values (2024-09-24T06:15:40.000+00:00,'shanghai','shanghai','huangpu','d01','red','A',40,40.0,true,'shanghai_huangpu_red_A_d01_40',2024-09-24T06:15:40.000+00:00)",
Expand Down Expand Up @@ -4377,26 +4378,26 @@ public void approxMostFrequentTest() {
expectedHeader = new String[] {"time", "province", "_col2"};
retArray =
new String[] {
"2024-09-24T06:15:30.000Z,beijing,{},",
"2024-09-24T06:15:30.000Z,beijing,null,",
"2024-09-24T06:15:31.000Z,beijing,{\"31000\":2},",
"2024-09-24T06:15:35.000Z,beijing,{\"35000\":2},",
"2024-09-24T06:15:36.000Z,beijing,{},",
"2024-09-24T06:15:36.000Z,beijing,null,",
"2024-09-24T06:15:40.000Z,beijing,{\"40000\":2},",
"2024-09-24T06:15:41.000Z,beijing,{},",
"2024-09-24T06:15:41.000Z,beijing,null,",
"2024-09-24T06:15:46.000Z,beijing,{\"46000\":2},",
"2024-09-24T06:15:50.000Z,beijing,{\"50000\":4},",
"2024-09-24T06:15:51.000Z,beijing,{},",
"2024-09-24T06:15:55.000Z,beijing,{},",
"2024-09-24T06:15:30.000Z,shanghai,{},",
"2024-09-24T06:15:51.000Z,beijing,null,",
"2024-09-24T06:15:55.000Z,beijing,null,",
"2024-09-24T06:15:30.000Z,shanghai,null,",
"2024-09-24T06:15:31.000Z,shanghai,{\"31000\":2},",
"2024-09-24T06:15:35.000Z,shanghai,{\"35000\":2},",
"2024-09-24T06:15:36.000Z,shanghai,{},",
"2024-09-24T06:15:36.000Z,shanghai,null,",
"2024-09-24T06:15:40.000Z,shanghai,{\"40000\":2},",
"2024-09-24T06:15:41.000Z,shanghai,{},",
"2024-09-24T06:15:41.000Z,shanghai,null,",
"2024-09-24T06:15:46.000Z,shanghai,{\"46000\":2},",
"2024-09-24T06:15:50.000Z,shanghai,{\"50000\":4},",
"2024-09-24T06:15:51.000Z,shanghai,{},",
"2024-09-24T06:15:55.000Z,shanghai,{},",
"2024-09-24T06:15:51.000Z,shanghai,null,",
"2024-09-24T06:15:55.000Z,shanghai,null,",
};

tableResultSetEqualTest(
Expand All @@ -4406,6 +4407,75 @@ public void approxMostFrequentTest() {
DATABASE_NAME);
}

@Test
public void blobGroupedApproxMostFrequentTest() {
// Groups whose BLOB column is entirely NULL must yield NULL instead of an empty map.
// Under a PARTIAL -> FINAL aggregation the partial side emits NULL for these groups and
// the final side never initializes their state, so evaluateFinal must handle that too.
tableResultSetEqualTest(
"select time,province,approx_most_frequent(s8, 1, 10) from table1 group by 1,2 order by 2,1",
new String[] {"time", "province", "_col2"},
new String[] {
"2024-09-24T06:15:30.000Z,beijing,{\"0xcafebabe30\":2},",
"2024-09-24T06:15:31.000Z,beijing,{\"0xcafebabe31\":2},",
"2024-09-24T06:15:35.000Z,beijing,null,",
"2024-09-24T06:15:36.000Z,beijing,null,",
"2024-09-24T06:15:40.000Z,beijing,null,",
"2024-09-24T06:15:41.000Z,beijing,{\"0xcafebabe41\":2},",
"2024-09-24T06:15:46.000Z,beijing,null,",
"2024-09-24T06:15:50.000Z,beijing,{\"0xcafebabe50\":2},",
"2024-09-24T06:15:51.000Z,beijing,null,",
"2024-09-24T06:15:55.000Z,beijing,{\"0xcafebabe55\":4},",
"2024-09-24T06:15:30.000Z,shanghai,{\"0xcafebabe30\":2},",
"2024-09-24T06:15:31.000Z,shanghai,{\"0xcafebabe31\":2},",
"2024-09-24T06:15:35.000Z,shanghai,null,",
"2024-09-24T06:15:36.000Z,shanghai,null,",
"2024-09-24T06:15:40.000Z,shanghai,null,",
"2024-09-24T06:15:41.000Z,shanghai,{\"0xcafebabe41\":2},",
"2024-09-24T06:15:46.000Z,shanghai,null,",
"2024-09-24T06:15:50.000Z,shanghai,{\"0xcafebabe50\":2},",
"2024-09-24T06:15:51.000Z,shanghai,null,",
"2024-09-24T06:15:55.000Z,shanghai,{\"0xcafebabe55\":4},",
},
DATABASE_NAME);
}

@Test
public void approxAggregationEmptyInputTest() {
// No block reaches the aggregation operator, so the accumulator state is never created
tableResultSetEqualTest(
"select approx_most_frequent(s1, 2, 10), approx_most_frequent(s2, 2, 10), approx_most_frequent(s6, 2, 10), approx_most_frequent(s8, 2, 10) from empty_table",
buildHeaders(4),
new String[] {"null,null,null,null,"},
DATABASE_NAME);

tableResultSetEqualTest(
"select approx_count_distinct(s1), approx_count_distinct(s6) from empty_table",
buildHeaders(2),
new String[] {"0,0,"},
DATABASE_NAME);

tableResultSetEqualTest(
"select device_id, approx_most_frequent(s1, 2, 10) from empty_table group by device_id",
new String[] {"device_id", "_col1"},
new String[] {},
DATABASE_NAME);

// All rows are filtered out by the predicate
tableResultSetEqualTest(
"select approx_most_frequent(s1, 2, 10), approx_count_distinct(s1) from table1 where s1 < 0",
buildHeaders(2),
new String[] {"null,0,"},
DATABASE_NAME);

// Rows exist but every value is null, so there is nothing to count
tableResultSetEqualTest(
"select approx_most_frequent(s2, 2, 10), approx_count_distinct(s2) from table1 where s2 is null",
buildHeaders(2),
new String[] {"null,0,"},
DATABASE_NAME);
}

@Test
public void approxPercentileTest() {
tableResultSetEqualTest(
Expand Down
19 changes: 16 additions & 3 deletions ...ecution/operator/source/relational/aggregation/AbstractApproxMostFrequentAccumulator.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@

package org.apache.iotdb.calc.execution.operator.source.relational.aggregation;

import org.apache.iotdb.calc.execution.operator.source.relational.aggregation.approximate.SpaceSaving;
import org.apache.iotdb.calc.execution.operator.source.relational.aggregation.approximate.SpaceSavingStateFactory;
import org.apache.iotdb.calc.i18n.CalcMessages;

Expand All @@ -36,13 +37,23 @@ public abstract class AbstractApproxMostFrequentAccumulator<T> implements TableA

@Override
public void evaluateIntermediate(ColumnBuilder columnBuilder) {
columnBuilder.writeBinary(new Binary(state.getSpaceSaving().serialize()));
SpaceSaving<T> spaceSaving = state.getSpaceSaving();
if (spaceSaving == null || spaceSaving.isEmpty()) {
columnBuilder.appendNull();
return;
}
columnBuilder.writeBinary(new Binary(spaceSaving.serialize()));
}

@Override
public void evaluateFinal(ColumnBuilder columnBuilder) {
SpaceSaving<T> spaceSaving = state.getSpaceSaving();
if (spaceSaving == null || spaceSaving.isEmpty()) {
columnBuilder.appendNull();
return;
}
columnBuilder.writeBinary(
new Binary(new Gson().toJson(state.getSpaceSaving().getBuckets()), StandardCharsets.UTF_8));
new Binary(new Gson().toJson(spaceSaving.getBuckets()), StandardCharsets.UTF_8));
}

@Override
Expand All @@ -58,6 +69,8 @@ public void addStatistics(Statistics[] statistics) {

@Override
public void reset() {
state.getSpaceSaving().reset();
if (state.getSpaceSaving() != null) {
state.getSpaceSaving().reset();
}
}
}
12 changes: 11 additions & 1 deletion ...calc/execution/operator/source/relational/aggregation/ApproxCountDistinctAccumulator.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -106,11 +106,19 @@ public void evaluateIntermediate(ColumnBuilder columnBuilder) {
columnBuilder instanceof BinaryColumnBuilder,
CalcMessages
.EXCEPTION_INTERMEDIATE_INPUT_AND_OUTPUT_OF_APPROX_COUNT_DISTINCT_SHOULD_BE_BINARYCOLUMN_F444D3BE);
if (state.getHyperLogLog() == null) {
columnBuilder.appendNull();
return;
}
columnBuilder.writeBinary(new Binary(state.getHyperLogLog().serialize()));
}

@Override
public void evaluateFinal(ColumnBuilder columnBuilder) {
if (state.getHyperLogLog() == null) {
columnBuilder.writeLong(0);
return;
}
columnBuilder.writeLong(state.getHyperLogLog().cardinality());
}

Expand All @@ -127,7 +135,9 @@ public void addStatistics(Statistics[] statistics) {

@Override
public void reset() {
state.getHyperLogLog().reset();
if (state.getHyperLogLog() != null) {
state.getHyperLogLog().reset();
}
}

public void addBooleanInput(Column valueColumn, AggregationMask mask, HyperLogLog hll) {
Expand Down
4 changes: 4 additions & 0 deletions ...c/execution/operator/source/relational/aggregation/BlobApproxMostFrequentAccumulator.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,10 @@
public class BlobApproxMostFrequentAccumulator extends BinaryApproxMostFrequentAccumulator {
@Override
public void evaluateFinal(ColumnBuilder columnBuilder) {
if (state.getSpaceSaving() == null || state.getSpaceSaving().isEmpty()) {
columnBuilder.appendNull();
return;
}
Map<Binary, Long> buckets = state.getSpaceSaving().getBuckets();
Map<String, Long> formatedBuckets =
buckets.entrySet().stream()
Expand Down
2 changes: 1 addition & 1 deletion ...execution/operator/source/relational/aggregation/approximate/HyperLogLogStateFactory.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ public void setHyperLogLog(HyperLogLog value) {

public long getEstimatedSize() {
// not used
return INSTANCE_SIZE + hll.getEstimatedSize();
return hll == null ? INSTANCE_SIZE : INSTANCE_SIZE + hll.getEstimatedSize();
}

public void merge(HyperLogLog other) {
Expand Down
4 changes: 4 additions & 0 deletions .../iotdb/calc/execution/operator/source/relational/aggregation/approximate/SpaceSaving.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -124,6 +124,10 @@ public Map<K, Long> getBuckets() {
return buckets.buildOrThrow();
}

public boolean isEmpty() {
return streamSummary.size() == 0;
}

public byte[] serialize() {
List<Counter<K>> counters = streamSummary.topK(capacity);
// Calculate the size of the keys
Expand Down
18 changes: 13 additions & 5 deletions ...r/source/relational/aggregation/grouped/AbstractGroupedApproxMostFrequentAccumulator.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@

package org.apache.iotdb.calc.execution.operator.source.relational.aggregation.grouped;

import org.apache.iotdb.calc.execution.operator.source.relational.aggregation.approximate.SpaceSaving;
import org.apache.iotdb.calc.execution.operator.source.relational.aggregation.approximate.SpaceSavingStateFactory;
import org.apache.iotdb.calc.execution.operator.source.relational.aggregation.grouped.array.SpaceSavingBigArray;

Expand All @@ -42,15 +43,22 @@ public void setGroupCount(long groupCount) {

@Override
public void evaluateIntermediate(int groupId, ColumnBuilder columnBuilder) {
columnBuilder.writeBinary(new Binary(state.getSpaceSavings().get(groupId).serialize()));
SpaceSaving<T> spaceSaving = state.getSpaceSavings().get(groupId);
if (spaceSaving == null || spaceSaving.isEmpty()) {
columnBuilder.appendNull();
return;
Comment on lines +47 to +49

@JackieTien97 JackieTien97 Nov 6, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P2] Handle empty BLOB groups in the overridden final evaluator

Please also add the null/empty-state guard to BlobGroupedApproxMostFrequentAccumulator.evaluateFinal(). For a group whose BLOB values are all NULL, this inherited evaluateIntermediate() now emits NULL, and BinaryGroupedApproxMostFrequentAccumulator.addIntermediate() skips it, leaving the final group's state uninitialized. The BLOB-specific evaluateFinal() overrides the guarded base implementation and still calls state.getSpaceSavings().get(groupId).getBuckets() unconditionally, so a PARTIAL -> FINAL aggregation now throws an NPE. I reproduced this with the parent and PR accumulator sources: the parent returns {}, whereas this revision throws. Please add a grouped BLOB regression test covering an all-NULL group through partial/final aggregation, including alongside a nonempty group.

}
columnBuilder.writeBinary(new Binary(spaceSaving.serialize()));
}

@Override
public void evaluateFinal(int groupId, ColumnBuilder columnBuilder) {
Binary result =
new Binary(
new Gson().toJson(state.getSpaceSavings().get(groupId).getBuckets()),
StandardCharsets.UTF_8);
SpaceSaving<T> spaceSaving = state.getSpaceSavings().get(groupId);
if (spaceSaving == null || spaceSaving.isEmpty()) {
columnBuilder.appendNull();
return;
}
Binary result = new Binary(new Gson().toJson(spaceSaving.getBuckets()), StandardCharsets.UTF_8);
columnBuilder.writeBinary(result);
}

Expand Down
9 changes: 8 additions & 1 deletion ...rator/source/relational/aggregation/grouped/BlobGroupedApproxMostFrequentAccumulator.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,8 @@

package org.apache.iotdb.calc.execution.operator.source.relational.aggregation.grouped;

import org.apache.iotdb.calc.execution.operator.source.relational.aggregation.approximate.SpaceSaving;

import com.google.gson.Gson;
import org.apache.tsfile.block.column.ColumnBuilder;
import org.apache.tsfile.utils.Binary;
Expand All @@ -32,7 +34,12 @@ public class BlobGroupedApproxMostFrequentAccumulator
extends BinaryGroupedApproxMostFrequentAccumulator {
@Override
public void evaluateFinal(int groupId, ColumnBuilder columnBuilder) {
Map<Binary, Long> buckets = state.getSpaceSavings().get(groupId).getBuckets();
SpaceSaving<Binary> spaceSaving = state.getSpaceSavings().get(groupId);
if (spaceSaving == null || spaceSaving.isEmpty()) {
columnBuilder.appendNull();
return;
}
Map<Binary, Long> buckets = spaceSaving.getBuckets();
Map<String, Long> formatedBuckets =
buckets.entrySet().stream()
.collect(
Expand Down
12 changes: 8 additions & 4 deletions ...operator/source/relational/aggregation/grouped/GroupedApproxCountDistinctAccumulator.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -101,14 +101,18 @@ public void addIntermediate(int[] groupIds, Column argument) {

@Override
public void evaluateIntermediate(int groupId, ColumnBuilder columnBuilder) {
HyperLogLogBigArray hlls = state.getHyperLogLogs();
columnBuilder.writeBinary(new Binary(hlls.get(groupId).serialize()));
HyperLogLog hll = state.getHyperLogLogs().get(groupId);
if (hll == null) {
columnBuilder.appendNull();
return;
}
columnBuilder.writeBinary(new Binary(hll.serialize()));
}

@Override
public void evaluateFinal(int groupId, ColumnBuilder columnBuilder) {
HyperLogLogBigArray hlls = state.getHyperLogLogs();
columnBuilder.writeLong(hlls.get(groupId).cardinality());
HyperLogLog hll = state.getHyperLogLogs().get(groupId);
columnBuilder.writeLong(hll == null ? 0 : hll.cardinality());
}

@Override
Expand Down
Loading