Skip to content

Commit

Permalink
Handle the case when enableNullHandling is true and an aggregation fu…
Browse files Browse the repository at this point in the history 
…nction is used w/ a column that has an empty null bitmap (#9566)
  • Loading branch information
@nizarhejazi
nizarhejazi authored Oct 11, 2022
1 parent 65e01f1 commit 6ab65f8
Show file tree
Hide file tree
Showing 4 changed files with 167 additions and 77 deletions.
31 changes: 17 additions & 14 deletions ...rc/main/java/org/apache/pinot/core/query/aggregation/function/MaxAggregationFunction.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -72,11 +72,13 @@ public void aggregate(int length, AggregationResultHolder aggregationResultHolde
Map<ExpressionContext, BlockValSet> blockValSetMap) {
BlockValSet blockValSet = blockValSetMap.get(_expression);
if (_nullHandlingEnabled) {
// TODO: avoid the null bitmap check when it is null or empty for better performance.
RoaringBitmap nullBitmap = blockValSet.getNullBitmap();
if (nullBitmap != null && !nullBitmap.isEmpty()) {
aggregateNullHandlingEnabled(length, aggregationResultHolder, blockValSet, nullBitmap);
return;
if (nullBitmap == null) {
nullBitmap = new RoaringBitmap();
}
aggregateNullHandlingEnabled(length, aggregationResultHolder, blockValSet, nullBitmap);
return;
}

switch (blockValSet.getValueType().getStoredType()) {
Expand Down Expand Up @@ -219,20 +221,21 @@ public void aggregateGroupBySV(int length, int[] groupKeyArray, GroupByResultHol
BlockValSet blockValSet = blockValSetMap.get(_expression);
if (_nullHandlingEnabled) {
RoaringBitmap nullBitmap = blockValSet.getNullBitmap();
if (nullBitmap != null && !nullBitmap.isEmpty()) {
if (nullBitmap.getCardinality() < length) {
double[] valueArray = blockValSet.getDoubleValuesSV();
for (int i = 0; i < length; i++) {
double value = valueArray[i];
int groupKey = groupKeyArray[i];
Double result = groupByResultHolder.getResult(groupKey);
if (!nullBitmap.contains(i) && (result == null || value > result)) {
groupByResultHolder.setValueForKey(groupKey, value);
}
if (nullBitmap == null) {
nullBitmap = new RoaringBitmap();
}
if (nullBitmap.getCardinality() < length) {
double[] valueArray = blockValSet.getDoubleValuesSV();
for (int i = 0; i < length; i++) {
double value = valueArray[i];
int groupKey = groupKeyArray[i];
Double result = groupByResultHolder.getResult(groupKey);
if (!nullBitmap.contains(i) && (result == null || value > result)) {
groupByResultHolder.setValueForKey(groupKey, value);
}
}
return;
}
return;
}

double[] valueArray = blockValSet.getDoubleValuesSV();
Expand Down
30 changes: 16 additions & 14 deletions ...rc/main/java/org/apache/pinot/core/query/aggregation/function/MinAggregationFunction.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -73,10 +73,11 @@ public void aggregate(int length, AggregationResultHolder aggregationResultHolde
BlockValSet blockValSet = blockValSetMap.get(_expression);
if (_nullHandlingEnabled) {
RoaringBitmap nullBitmap = blockValSet.getNullBitmap();
if (nullBitmap != null && !nullBitmap.isEmpty()) {
aggregateNullHandlingEnabled(length, aggregationResultHolder, blockValSet, nullBitmap);
return;
if (nullBitmap == null) {
nullBitmap = new RoaringBitmap();
}
aggregateNullHandlingEnabled(length, aggregationResultHolder, blockValSet, nullBitmap);
return;
}

switch (blockValSet.getValueType().getStoredType()) {
Expand Down Expand Up @@ -219,20 +220,21 @@ public void aggregateGroupBySV(int length, int[] groupKeyArray, GroupByResultHol
BlockValSet blockValSet = blockValSetMap.get(_expression);
if (_nullHandlingEnabled) {
RoaringBitmap nullBitmap = blockValSet.getNullBitmap();
if (nullBitmap != null && !nullBitmap.isEmpty()) {
if (nullBitmap.getCardinality() < length) {
double[] valueArray = blockValSet.getDoubleValuesSV();
for (int i = 0; i < length; i++) {
double value = valueArray[i];
int groupKey = groupKeyArray[i];
Double result = groupByResultHolder.getResult(groupKey);
if (!nullBitmap.contains(i) && (result == null || value < result)) {
groupByResultHolder.setValueForKey(groupKey, value);
}
if (nullBitmap == null) {
nullBitmap = new RoaringBitmap();
}
if (nullBitmap.getCardinality() < length) {
double[] valueArray = blockValSet.getDoubleValuesSV();
for (int i = 0; i < length; i++) {
double value = valueArray[i];
int groupKey = groupKeyArray[i];
Double result = groupByResultHolder.getResult(groupKey);
if (!nullBitmap.contains(i) && (result == null || value < result)) {
groupByResultHolder.setValueForKey(groupKey, value);
}
}
return;
}
return;
}

double[] valueArray = blockValSet.getDoubleValuesSV();
Expand Down
42 changes: 22 additions & 20 deletions ...rc/main/java/org/apache/pinot/core/query/aggregation/function/SumAggregationFunction.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -73,10 +73,11 @@ public void aggregate(int length, AggregationResultHolder aggregationResultHolde
BlockValSet blockValSet = blockValSetMap.get(_expression);
if (_nullHandlingEnabled) {
RoaringBitmap nullBitmap = blockValSet.getNullBitmap();
if (nullBitmap != null && !nullBitmap.isEmpty()) {
aggregateNullHandlingEnabled(length, aggregationResultHolder, blockValSet, nullBitmap);
return;
if (nullBitmap == null) {
nullBitmap = new RoaringBitmap();
}
aggregateNullHandlingEnabled(length, aggregationResultHolder, blockValSet, nullBitmap);
return;
}

double sum = aggregationResultHolder.getDoubleResult();
Expand Down Expand Up @@ -207,26 +208,27 @@ public void aggregateGroupBySV(int length, int[] groupKeyArray, GroupByResultHol
BlockValSet blockValSet = blockValSetMap.get(_expression);
if (_nullHandlingEnabled) {
RoaringBitmap nullBitmap = blockValSet.getNullBitmap();
if (nullBitmap != null && !nullBitmap.isEmpty()) {
if (nullBitmap.getCardinality() < length) {
double[] valueArray = blockValSet.getDoubleValuesSV();
for (int i = 0; i < length; i++) {
if (!nullBitmap.contains(i)) {
int groupKey = groupKeyArray[i];
Double result = groupByResultHolder.getResult(groupKey);
groupByResultHolder.setValueForKey(groupKey, result == null ? valueArray[i] : result + valueArray[i]);
// In presto:
// SELECT sum (cast(id AS DOUBLE)) as sum, min(id) as min, max(id) as max, key FROM (VALUES (null, 1),
// (null, 2)) AS t(id, key) GROUP BY key ORDER BY max DESC;
// sum | min | max | key
//------+------+------+-----
// NULL | NULL | NULL | 2
// NULL | NULL | NULL | 1
}
if (nullBitmap == null) {
nullBitmap = new RoaringBitmap();
}
if (nullBitmap.getCardinality() < length) {
double[] valueArray = blockValSet.getDoubleValuesSV();
for (int i = 0; i < length; i++) {
if (!nullBitmap.contains(i)) {
int groupKey = groupKeyArray[i];
Double result = groupByResultHolder.getResult(groupKey);
groupByResultHolder.setValueForKey(groupKey, result == null ? valueArray[i] : result + valueArray[i]);
// In presto:
// SELECT sum (cast(id AS DOUBLE)) as sum, min(id) as min, max(id) as max, key FROM (VALUES (null, 1),
// (null, 2)) AS t(id, key) GROUP BY key ORDER BY max DESC;
// sum | min | max | key
//------+------+------+-----
// NULL | NULL | NULL | 2
// NULL | NULL | NULL | 1
}
}
return;
}
return;
}

double[] valueArray = blockValSet.getDoubleValuesSV();
Expand Down
Loading

0 comments on commit 6ab65f8

Please sign in to comment.