Skip to content

Commit

Permalink
SQL: Fix result values for COUNT(DISTINCT ...) (#68666) (#68732)
Browse files Browse the repository at this point in the history
Previously, we extracted the result of the `CardinalityAgg` as `double`
which resulted in values shown in the REST response with `.0`, even though
the type of the corresponding column for `COUNT(DISTINCT <field_name>)`
was showing `long`, e.g.: `152.0` instead of `152`.
This affected only the REST interface of SQL and not the JDBC/ODBC drivers.

Extract a long value instead of a double.

Fixes: #58097
(cherry picked from commit 4660fae)
  • Loading branch information
matriv committed Feb 9, 2021
1 parent cf0db46 commit e34a8f2
Show file tree
Hide file tree
Showing 2 changed files with 47 additions and 1 deletion.
Original file line number Diff line number Diff line change
Expand Up @@ -418,6 +418,50 @@ public void testSelectGroupByScore() throws Exception {
);
}

public void testCountAndCountDistinct() throws IOException {
String mode = randomMode();
index(
"test",
"{\"gender\":\"m\", \"langs\": 1}",
"{\"gender\":\"m\", \"langs\": 1}",
"{\"gender\":\"m\", \"langs\": 2}",
"{\"gender\":\"m\", \"langs\": 3}",
"{\"gender\":\"m\", \"langs\": 3}",
"{\"gender\":\"f\", \"langs\": 1}",
"{\"gender\":\"f\", \"langs\": 2}",
"{\"gender\":\"f\", \"langs\": 2}",
"{\"gender\":\"f\", \"langs\": 2}",
"{\"gender\":\"f\", \"langs\": 3}",
"{\"gender\":\"f\", \"langs\": 3}"
);

Map<String, Object> expected = new HashMap<>();
boolean columnar = randomBoolean();
expected.put(
"columns",
Arrays.asList(
columnInfo(mode, "gender", "text", JDBCType.VARCHAR, Integer.MAX_VALUE),
columnInfo(mode, "cnt", "long", JDBCType.BIGINT, 20),
columnInfo(mode, "cnt_dist", "long", JDBCType.BIGINT, 20)
)
);
if (columnar) {
expected.put("values", Arrays.asList(Arrays.asList("f", "m"), Arrays.asList(6, 5), Arrays.asList(3, 3)));
} else {
expected.put("rows", Arrays.asList(Arrays.asList("f", 6, 3), Arrays.asList("m", 5, 3)));
}

Map<String, Object> response = runSql(
mode,
"SELECT gender, COUNT(langs) AS cnt, COUNT(DISTINCT langs) AS cnt_dist " + "FROM test GROUP BY gender ORDER BY gender",
columnar
);

String cursor = (String) response.remove("cursor");
assertNotNull(cursor);
assertResponse(expected, response);
}

@Override
public void testSelectScoreSubField() throws Exception {
index("{\"foo\":1}");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
import org.elasticsearch.search.aggregations.bucket.filter.InternalFilter;
import org.elasticsearch.search.aggregations.matrix.stats.InternalMatrixStats;
import org.elasticsearch.search.aggregations.metrics.InternalAvg;
import org.elasticsearch.search.aggregations.metrics.InternalCardinality;
import org.elasticsearch.search.aggregations.metrics.InternalMax;
import org.elasticsearch.search.aggregations.metrics.InternalMin;
import org.elasticsearch.search.aggregations.metrics.InternalNumericMetricsAggregation;
Expand All @@ -25,7 +26,6 @@
import org.elasticsearch.xpack.sql.common.io.SqlStreamInput;
import org.elasticsearch.xpack.sql.querydsl.agg.Aggs;
import org.elasticsearch.xpack.sql.util.DateUtils;

import java.io.IOException;
import java.time.ZoneId;
import java.util.Map;
Expand Down Expand Up @@ -110,6 +110,8 @@ public Object extract(Bucket bucket) {
} else if (agg instanceof InternalFilter) {
// COUNT(expr) and COUNT(ALL expr) uses this type of aggregation to account for non-null values only
return ((InternalFilter) agg).getDocCount();
} else if (agg instanceof InternalCardinality) {
return ((InternalCardinality) agg).getValue();
}

Object v = agg.getProperty(property);
Expand Down

0 comments on commit e34a8f2

Please sign in to comment.