Skip to content

Commit c4cc3d5

Browse files
committed
SQL: Handle aggregation for null group (#34916)
When dealing with a null group, the associated metric aggs need to return null as well Fix #34896 (cherry picked from commit 55d48e4)
1 parent cb20f9e commit c4cc3d5

File tree

3 files changed

+54
-1
lines changed

3 files changed

+54
-1
lines changed

x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/execution/search/Querier.java

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -253,8 +253,9 @@ protected List<BucketExtractor> initBucketExtractors(SearchResponse response) {
253253
List<FieldExtraction> refs = query.columns();
254254

255255
List<BucketExtractor> exts = new ArrayList<>(refs.size());
256+
ConstantExtractor totalCount = new ConstantExtractor(response.getHits().getTotalHits());
256257
for (FieldExtraction ref : refs) {
257-
exts.add(createExtractor(ref, new ConstantExtractor(response.getHits().getTotalHits())));
258+
exts.add(createExtractor(ref, totalCount));
258259
}
259260
return exts;
260261
}

x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/execution/search/extractor/MetricAggExtractor.java

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,12 @@
99
import org.elasticsearch.common.io.stream.StreamOutput;
1010
import org.elasticsearch.search.aggregations.InternalAggregation;
1111
import org.elasticsearch.search.aggregations.bucket.MultiBucketsAggregation.Bucket;
12+
import org.elasticsearch.search.aggregations.matrix.stats.MatrixStats;
1213
import org.elasticsearch.search.aggregations.metrics.InternalNumericMetricsAggregation;
14+
import org.elasticsearch.search.aggregations.metrics.InternalNumericMetricsAggregation.SingleValue;
15+
import org.elasticsearch.search.aggregations.metrics.percentiles.PercentileRanks;
16+
import org.elasticsearch.search.aggregations.metrics.percentiles.Percentiles;
17+
import org.elasticsearch.search.aggregations.metrics.stats.InternalStats;
1318
import org.elasticsearch.xpack.sql.SqlIllegalArgumentException;
1419
import org.elasticsearch.xpack.sql.querydsl.agg.Aggs;
1520

@@ -67,6 +72,11 @@ public Object extract(Bucket bucket) {
6772
if (agg == null) {
6873
throw new SqlIllegalArgumentException("Cannot find an aggregation named {}", name);
6974
}
75+
76+
if (!containsValues(agg)) {
77+
return null;
78+
}
79+
7080
if (agg instanceof InternalNumericMetricsAggregation.MultiValue) {
7181
//TODO: need to investigate when this can be not-null
7282
//if (innerKey == null) {
@@ -79,6 +89,33 @@ public Object extract(Bucket bucket) {
7989
return innerKey != null && v instanceof Map ? ((Map<?, ?>) v).get(innerKey) : v;
8090
}
8191

92+
/**
93+
* Check if the given aggregate has been executed and has computed values
94+
* or not (the bucket is null).
95+
*
96+
* Waiting on https://github.com/elastic/elasticsearch/issues/34903
97+
*/
98+
private static boolean containsValues(InternalAggregation agg) {
99+
// Stats & ExtendedStats
100+
if (agg instanceof InternalStats) {
101+
return ((InternalStats) agg).getCount() != 0;
102+
}
103+
if (agg instanceof MatrixStats) {
104+
return ((MatrixStats) agg).getDocCount() != 0;
105+
}
106+
// sum returns 0 even for null; since that's a common case, we return it as such
107+
if (agg instanceof SingleValue) {
108+
return Double.isFinite(((SingleValue) agg).value());
109+
}
110+
if (agg instanceof PercentileRanks) {
111+
return Double.isFinite(((PercentileRanks) agg).percent(0));
112+
}
113+
if (agg instanceof Percentiles) {
114+
return Double.isFinite(((Percentiles) agg).percentile(0));
115+
}
116+
return true;
117+
}
118+
82119
@Override
83120
public int hashCode() {
84121
return Objects.hash(name, property, innerKey);

x-pack/qa/sql/src/main/resources/agg.csv-spec

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -121,4 +121,19 @@ gender:s | k:d | s:d
121121
null |2.2215791166941923 |-0.03373126000214023
122122
F |1.7873117044424276 |0.05504995122217512
123123
M |2.280646181070106 |0.44302407229580243
124+
;
125+
126+
nullAggs
127+
SELECT MAX(languages) max, MIN(languages) min, SUM(languages) sum, AVG(languages) avg,
128+
PERCENTILE(languages, 80) percent, PERCENTILE_RANK(languages, 3) percent_rank,
129+
KURTOSIS(languages) kurtosis, SKEWNESS(languages) skewness
130+
FROM test_emp GROUP BY languages ORDER BY languages ASC LIMIT 5;
131+
132+
max:bt | min:bt | sum:bt | avg:bt | percent:d | percent_rank:d| kurtosis:d | skewness:d
133+
---------------+---------------+---------------+---------------+---------------+---------------+---------------+---------------
134+
null |null |null |null |null |null |null |null
135+
1 |1 |15 |1 |1.0 |100.0 |NaN |NaN
136+
2 |2 |38 |2 |2.0 |100.0 |NaN |NaN
137+
3 |3 |51 |3 |3.0 |100.0 |NaN |NaN
138+
4 |4 |72 |4 |4.0 |0.0 |NaN |NaN
124139
;

0 commit comments

Comments
 (0)