Skip to content

Commit

Permalink
SQL: Handle aggregation for null group (#34916)
Browse files Browse the repository at this point in the history
When dealing with a null group, the associated metric aggs need to
return null as well

Fix #34896
  • Loading branch information
costin authored and kcm committed Oct 30, 2018
1 parent 059e878 commit 056dac3
Show file tree
Hide file tree
Showing 3 changed files with 54 additions and 1 deletion.
Expand Up @@ -253,8 +253,9 @@ protected List<BucketExtractor> initBucketExtractors(SearchResponse response) {
List<FieldExtraction> refs = query.columns();

List<BucketExtractor> exts = new ArrayList<>(refs.size());
ConstantExtractor totalCount = new ConstantExtractor(response.getHits().getTotalHits());
for (FieldExtraction ref : refs) {
exts.add(createExtractor(ref, new ConstantExtractor(response.getHits().getTotalHits())));
exts.add(createExtractor(ref, totalCount));
}
return exts;
}
Expand Down
Expand Up @@ -9,7 +9,12 @@
import org.elasticsearch.common.io.stream.StreamOutput;
import org.elasticsearch.search.aggregations.InternalAggregation;
import org.elasticsearch.search.aggregations.bucket.MultiBucketsAggregation.Bucket;
import org.elasticsearch.search.aggregations.matrix.stats.MatrixStats;
import org.elasticsearch.search.aggregations.metrics.InternalNumericMetricsAggregation;
import org.elasticsearch.search.aggregations.metrics.InternalNumericMetricsAggregation.SingleValue;
import org.elasticsearch.search.aggregations.metrics.InternalStats;
import org.elasticsearch.search.aggregations.metrics.PercentileRanks;
import org.elasticsearch.search.aggregations.metrics.Percentiles;
import org.elasticsearch.xpack.sql.SqlIllegalArgumentException;
import org.elasticsearch.xpack.sql.querydsl.agg.Aggs;

Expand Down Expand Up @@ -67,6 +72,11 @@ public Object extract(Bucket bucket) {
if (agg == null) {
throw new SqlIllegalArgumentException("Cannot find an aggregation named {}", name);
}

if (!containsValues(agg)) {
return null;
}

if (agg instanceof InternalNumericMetricsAggregation.MultiValue) {
//TODO: need to investigate when this can be not-null
//if (innerKey == null) {
Expand All @@ -79,6 +89,33 @@ public Object extract(Bucket bucket) {
return innerKey != null && v instanceof Map ? ((Map<?, ?>) v).get(innerKey) : v;
}

/**
* Check if the given aggregate has been executed and has computed values
* or not (the bucket is null).
*
* Waiting on https://github.com/elastic/elasticsearch/issues/34903
*/
private static boolean containsValues(InternalAggregation agg) {
// Stats & ExtendedStats
if (agg instanceof InternalStats) {
return ((InternalStats) agg).getCount() != 0;
}
if (agg instanceof MatrixStats) {
return ((MatrixStats) agg).getDocCount() != 0;
}
// sum returns 0 even for null; since that's a common case, we return it as such
if (agg instanceof SingleValue) {
return Double.isFinite(((SingleValue) agg).value());
}
if (agg instanceof PercentileRanks) {
return Double.isFinite(((PercentileRanks) agg).percent(0));
}
if (agg instanceof Percentiles) {
return Double.isFinite(((Percentiles) agg).percentile(0));
}
return true;
}

@Override
public int hashCode() {
return Objects.hash(name, property, innerKey);
Expand Down
15 changes: 15 additions & 0 deletions x-pack/qa/sql/src/main/resources/agg.csv-spec
Expand Up @@ -121,4 +121,19 @@ gender:s | k:d | s:d
null |2.2215791166941923 |-0.03373126000214023
F |1.7873117044424276 |0.05504995122217512
M |2.280646181070106 |0.44302407229580243
;

nullAggs
SELECT MAX(languages) max, MIN(languages) min, SUM(languages) sum, AVG(languages) avg,
PERCENTILE(languages, 80) percent, PERCENTILE_RANK(languages, 3) percent_rank,
KURTOSIS(languages) kurtosis, SKEWNESS(languages) skewness
FROM test_emp GROUP BY languages ORDER BY languages ASC LIMIT 5;

max:bt | min:bt | sum:bt | avg:bt | percent:d | percent_rank:d| kurtosis:d | skewness:d
---------------+---------------+---------------+---------------+---------------+---------------+---------------+---------------
null |null |null |null |null |null |null |null
1 |1 |15 |1 |1.0 |100.0 |NaN |NaN
2 |2 |38 |2 |2.0 |100.0 |NaN |NaN
3 |3 |51 |3 |3.0 |100.0 |NaN |NaN
4 |4 |72 |4 |4.0 |0.0 |NaN |NaN
;

0 comments on commit 056dac3

Please sign in to comment.