Skip to content

Commit e56a463

Browse files
committed
SQL: Fix issue with GROUP BY YEAR() (#49559)
Grouping By YEAR() is translated to a histogram aggregation, but previously if there was a scalar function invloved (e.g.: `YEAR(date + INTERVAL 2 YEARS)`), there was no proper script created and the histogram was applied on a field with name: `date + INTERVAL 2 YEARS` which doesn't make sense, and resulted in null result. Check the underlying field of YEAR() and if it's a function call `asScript()` to properly get the painless script on which the histogram is applied. Fixes: #49386 (cherry picked from commit 93c37ab)
1 parent 05eb6f3 commit e56a463

File tree

3 files changed

+53
-4
lines changed

3 files changed

+53
-4
lines changed

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

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -534,6 +534,29 @@ SELECT HISTOGRAM(YEAR(birth_date), 2) AS h, COUNT(*) as c FROM test_emp GROUP BY
534534
null |10
535535
;
536536

537+
histogramYearOnDateTimeWithScalars
538+
schema::year:i|c:l
539+
SELECT YEAR(CAST(birth_date + INTERVAL 5 YEARS AS DATE) + INTERVAL 20 MONTHS) AS year, COUNT(*) as c FROM test_emp GROUP BY 1;
540+
541+
year | c
542+
---------------+---------------
543+
null |10
544+
1958 |2
545+
1959 |12
546+
1960 |7
547+
1961 |7
548+
1962 |4
549+
1963 |5
550+
1964 |5
551+
1965 |7
552+
1966 |9
553+
1967 |7
554+
1968 |7
555+
1969 |8
556+
1970 |6
557+
1971 |4
558+
;
559+
537560
histogramNumericWithExpression
538561
schema::h:i|c:l
539562
SELECT HISTOGRAM(emp_no % 100, 10) AS h, COUNT(*) as c FROM test_emp GROUP BY h ORDER BY h DESC;

x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/planner/QueryTranslator.java

Lines changed: 14 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -281,10 +281,20 @@ static GroupingContext groupBy(List<? extends Expression> groupings) {
281281
// dates are handled differently because of date histograms
282282
if (exp instanceof DateTimeHistogramFunction) {
283283
DateTimeHistogramFunction dthf = (DateTimeHistogramFunction) exp;
284-
if (dthf.calendarInterval() != null) {
285-
key = new GroupByDateHistogram(aggId, nameOf(exp), dthf.calendarInterval(), dthf.zoneId());
286-
} else {
287-
key = new GroupByDateHistogram(aggId, nameOf(exp), dthf.fixedInterval(), dthf.zoneId());
284+
Expression field = dthf.field();
285+
if (field instanceof FieldAttribute) {
286+
if (dthf.calendarInterval() != null) {
287+
key = new GroupByDateHistogram(aggId, nameOf(field), dthf.calendarInterval(), dthf.zoneId());
288+
} else {
289+
key = new GroupByDateHistogram(aggId, nameOf(field), dthf.fixedInterval(), dthf.zoneId());
290+
}
291+
} else if (field instanceof Function) {
292+
ScriptTemplate script = ((Function) field).asScript();
293+
if (dthf.calendarInterval() != null) {
294+
key = new GroupByDateHistogram(aggId, script, dthf.calendarInterval(), dthf.zoneId());
295+
} else {
296+
key = new GroupByDateHistogram(aggId, script, dthf.fixedInterval(), dthf.zoneId());
297+
}
288298
}
289299
}
290300
// all other scalar functions become a script

x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/planner/QueryTranslatorTests.java

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -952,6 +952,22 @@ public void testGroupByYearQueryTranslator() {
952952
+ "\"calendar_interval\":\"1y\",\"time_zone\":\"Z\"}}}]}}}"));
953953
}
954954

955+
public void testGroupByYearAndScalarsQueryTranslator() {
956+
PhysicalPlan p = optimizeAndPlan("SELECT YEAR(CAST(date + INTERVAL 5 months AS DATE)) FROM test GROUP BY 1");
957+
assertEquals(EsQueryExec.class, p.getClass());
958+
EsQueryExec eqe = (EsQueryExec) p;
959+
assertEquals(1, eqe.output().size());
960+
assertEquals("YEAR(CAST(date + INTERVAL 5 months AS DATE))", eqe.output().get(0).qualifiedName());
961+
assertEquals(DataType.INTEGER, eqe.output().get(0).dataType());
962+
assertThat(eqe.queryContainer().aggs().asAggBuilder().toString().replaceAll("\\s+", ""),
963+
endsWith("\"date_histogram\":{\"script\":{\"source\":\"InternalSqlScriptUtils.cast(" +
964+
"InternalSqlScriptUtils.add(InternalSqlScriptUtils.docValue(doc,params.v0)," +
965+
"InternalSqlScriptUtils.intervalYearMonth(params.v1,params.v2)),params.v3)\"," +
966+
"\"lang\":\"painless\",\"params\":{\"v0\":\"date\",\"v1\":\"P5M\",\"v2\":\"INTERVAL_MONTH\"," +
967+
"\"v3\":\"DATE\"}},\"missing_bucket\":true,\"value_type\":\"long\",\"order\":\"asc\"," +
968+
"\"calendar_interval\":\"1y\",\"time_zone\":\"Z\"}}}]}}}"));
969+
}
970+
955971
public void testGroupByHistogramWithDate() {
956972
LogicalPlan p = plan("SELECT MAX(int) FROM test GROUP BY HISTOGRAM(CAST(date AS DATE), INTERVAL 2 MONTHS)");
957973
assertTrue(p instanceof Aggregate);

0 commit comments

Comments
 (0)