Skip to content

Commit 7cf100b

Browse files
astefanastefan
committed
SQL: fix scripting for grouped by datetime functions (#46421)
* Fix issue with painless scripting not being correctly generated when datetime functions are used for GROUPing of an INTERVAL operation. (cherry picked from commit cb92828)
1 parent 1bb1c77 commit 7cf100b

File tree

6 files changed

+156
-32
lines changed

6 files changed

+156
-32
lines changed

‎x-pack/plugin/sql/qa/src/main/resources/datetime-interval.csv-spec

Lines changed: 94 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -309,3 +309,97 @@ SELECT birth_date, MAX(hire_date) - INTERVAL 1 YEAR AS f FROM test_emp GROUP BY
309309
1952-06-13T00:00:00Z|1953
310310
1952-07-08T00:00:00Z|1953
311311
;
312+
313+
monthOfDatePlusInterval_And_GroupBy
314+
SELECT WEEK_OF_YEAR(birth_date + INTERVAL 25 YEAR) x, COUNT(*) c FROM test_emp GROUP BY x HAVING c >= 3 ORDER BY c DESC;
315+
316+
x:i | c:l
317+
---------------+---------------
318+
null |10
319+
22 |6
320+
4 |4
321+
16 |4
322+
30 |4
323+
40 |4
324+
45 |4
325+
1 |3
326+
8 |3
327+
21 |3
328+
28 |3
329+
32 |3
330+
37 |3
331+
;
332+
333+
dayOfWeekPlusInterval_And_GroupBy
334+
SELECT DOW(birth_date + INTERVAL 5 YEAR) x, COUNT(*) c FROM test_emp GROUP BY x HAVING c >= 3 ORDER BY c DESC;
335+
336+
x:i | c:l
337+
---------------+---------------
338+
2 |18
339+
3 |15
340+
5 |15
341+
4 |12
342+
6 |12
343+
null |10
344+
7 |10
345+
1 |8
346+
;
347+
348+
dayNamePlusInterval_And_GroupBy
349+
SELECT DAY_NAME(birth_date + INTERVAL 5 YEAR) x, COUNT(*) c FROM test_emp GROUP BY x HAVING c >= 10 ORDER BY c DESC;
350+
351+
x:s | c:l
352+
---------------+---------------
353+
Monday |18
354+
Thursday |15
355+
Tuesday |15
356+
Friday |12
357+
Wednesday |12
358+
null |10
359+
Saturday |10
360+
;
361+
362+
monthNamePlusInterval_And_GroupBy
363+
SELECT MONTH_NAME(birth_date + INTERVAL 5 YEAR) x, COUNT(*) c FROM test_emp GROUP BY x HAVING c >= 5 ORDER BY c DESC;
364+
365+
x:s | c:l
366+
---------------+---------------
367+
null |10
368+
May |10
369+
September |10
370+
July |9
371+
October |9
372+
April |8
373+
February |8
374+
November |8
375+
December |7
376+
June |7
377+
August |6
378+
January |6
379+
;
380+
381+
quarterPlusInterval_And_GroupBy
382+
SELECT QUARTER(birth_date + INTERVAL 5 YEAR) x, COUNT(*) c FROM test_emp GROUP BY x HAVING c >= 5 ORDER BY x DESC;
383+
384+
x:i | c:l
385+
---------------+---------------
386+
4 |24
387+
3 |25
388+
2 |25
389+
1 |16
390+
null |10
391+
;
392+
393+
dayOfMonthPlusInterval_And_GroupBy
394+
SELECT DOM(birth_date + INTERVAL 5 YEAR) x, COUNT(*) c FROM test_emp GROUP BY x HAVING c >= 5 ORDER BY x DESC;
395+
396+
x:i | c:l
397+
---------------+---------------
398+
25 |5
399+
23 |6
400+
21 |5
401+
19 |7
402+
7 |5
403+
1 |5
404+
null |10
405+
;

‎x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/function/scalar/datetime/DateTimeFunction.java

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,6 @@ public ScriptTemplate asScript() {
4949
.variable(extractor.chronoField().name());
5050

5151
return new ScriptTemplate(template, params.build(), dataType());
52-
5352
}
5453

5554
@Override

‎x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/function/scalar/datetime/NamedDateTimeFunction.java

Lines changed: 9 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -6,18 +6,16 @@
66
package org.elasticsearch.xpack.sql.expression.function.scalar.datetime;
77

88
import org.elasticsearch.xpack.sql.expression.Expression;
9-
import org.elasticsearch.xpack.sql.expression.FieldAttribute;
109
import org.elasticsearch.xpack.sql.expression.function.scalar.datetime.NamedDateTimeProcessor.NameExtractor;
1110
import org.elasticsearch.xpack.sql.expression.gen.processor.Processor;
11+
import org.elasticsearch.xpack.sql.expression.gen.script.ParamsBuilder;
1212
import org.elasticsearch.xpack.sql.expression.gen.script.ScriptTemplate;
1313
import org.elasticsearch.xpack.sql.tree.Source;
1414
import org.elasticsearch.xpack.sql.type.DataType;
1515
import org.elasticsearch.xpack.sql.util.StringUtils;
1616

1717
import java.time.ZoneId;
18-
import java.util.Locale;
1918

20-
import static java.lang.String.format;
2119
import static org.elasticsearch.xpack.sql.expression.gen.script.ParamsBuilder.paramsBuilder;
2220

2321
/*
@@ -33,14 +31,14 @@ abstract class NamedDateTimeFunction extends BaseDateTimeFunction {
3331
}
3432

3533
@Override
36-
public ScriptTemplate scriptWithField(FieldAttribute field) {
37-
return new ScriptTemplate(
38-
formatTemplate(format(Locale.ROOT, "{sql}.%s(doc[{}].value, {})",
39-
StringUtils.underscoreToLowerCamelCase(nameExtractor.name()))),
40-
paramsBuilder()
41-
.variable(field.name())
42-
.variable(zoneId().getId()).build(),
43-
dataType());
34+
public ScriptTemplate asScript() {
35+
ScriptTemplate script = super.asScript();
36+
String template = formatTemplate("{sql}." + StringUtils.underscoreToLowerCamelCase(nameExtractor.name())
37+
+ "(" + script.template() + ", {})");
38+
39+
ParamsBuilder params = paramsBuilder().script(script.params()).variable(zoneId().getId());
40+
41+
return new ScriptTemplate(template, params.build(), dataType());
4442
}
4543

4644
@Override

‎x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/function/scalar/datetime/NonIsoDateTimeFunction.java

Lines changed: 9 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -6,18 +6,16 @@
66
package org.elasticsearch.xpack.sql.expression.function.scalar.datetime;
77

88
import org.elasticsearch.xpack.sql.expression.Expression;
9-
import org.elasticsearch.xpack.sql.expression.FieldAttribute;
109
import org.elasticsearch.xpack.sql.expression.function.scalar.datetime.NonIsoDateTimeProcessor.NonIsoDateTimeExtractor;
1110
import org.elasticsearch.xpack.sql.expression.gen.processor.Processor;
11+
import org.elasticsearch.xpack.sql.expression.gen.script.ParamsBuilder;
1212
import org.elasticsearch.xpack.sql.expression.gen.script.ScriptTemplate;
1313
import org.elasticsearch.xpack.sql.tree.Source;
1414
import org.elasticsearch.xpack.sql.type.DataType;
1515
import org.elasticsearch.xpack.sql.util.StringUtils;
1616

1717
import java.time.ZoneId;
18-
import java.util.Locale;
1918

20-
import static java.lang.String.format;
2119
import static org.elasticsearch.xpack.sql.expression.gen.script.ParamsBuilder.paramsBuilder;
2220

2321
/*
@@ -33,14 +31,14 @@ abstract class NonIsoDateTimeFunction extends BaseDateTimeFunction {
3331
}
3432

3533
@Override
36-
public ScriptTemplate scriptWithField(FieldAttribute field) {
37-
return new ScriptTemplate(
38-
formatTemplate(format(Locale.ROOT, "{sql}.%s(doc[{}].value, {})",
39-
StringUtils.underscoreToLowerCamelCase(extractor.name()))),
40-
paramsBuilder()
41-
.variable(field.name())
42-
.variable(zoneId().getId()).build(),
43-
dataType());
34+
public ScriptTemplate asScript() {
35+
ScriptTemplate script = super.asScript();
36+
String template = formatTemplate("{sql}." + StringUtils.underscoreToLowerCamelCase(extractor.name())
37+
+ "(" + script.template() + ", {})");
38+
39+
ParamsBuilder params = paramsBuilder().script(script.params()).variable(zoneId().getId());
40+
41+
return new ScriptTemplate(template, params.build(), dataType());
4442
}
4543

4644
@Override

‎x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/function/scalar/datetime/Quarter.java

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -7,8 +7,8 @@
77
package org.elasticsearch.xpack.sql.expression.function.scalar.datetime;
88

99
import org.elasticsearch.xpack.sql.expression.Expression;
10-
import org.elasticsearch.xpack.sql.expression.FieldAttribute;
1110
import org.elasticsearch.xpack.sql.expression.gen.processor.Processor;
11+
import org.elasticsearch.xpack.sql.expression.gen.script.ParamsBuilder;
1212
import org.elasticsearch.xpack.sql.expression.gen.script.ScriptTemplate;
1313
import org.elasticsearch.xpack.sql.tree.NodeInfo.NodeCtor2;
1414
import org.elasticsearch.xpack.sql.tree.Source;
@@ -23,15 +23,15 @@ public class Quarter extends BaseDateTimeFunction {
2323
public Quarter(Source source, Expression field, ZoneId zoneId) {
2424
super(source, field, zoneId);
2525
}
26-
26+
2727
@Override
28-
public ScriptTemplate scriptWithField(FieldAttribute field) {
29-
return new ScriptTemplate(formatTemplate("{sql}.quarter(doc[{}].value, {})"),
30-
paramsBuilder()
31-
.variable(field.name())
32-
.variable(zoneId().getId())
33-
.build(),
34-
dataType());
28+
public ScriptTemplate asScript() {
29+
ScriptTemplate script = super.asScript();
30+
String template = formatTemplate("{sql}.quarter(" + script.template() + ", {})");
31+
32+
ParamsBuilder params = paramsBuilder().script(script.params()).variable(zoneId().getId());
33+
34+
return new ScriptTemplate(template, params.build(), dataType());
3535
}
3636

3737
@Override

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

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@
2424
import org.elasticsearch.xpack.sql.expression.function.FunctionRegistry;
2525
import org.elasticsearch.xpack.sql.expression.function.grouping.Histogram;
2626
import org.elasticsearch.xpack.sql.expression.function.scalar.Cast;
27+
import org.elasticsearch.xpack.sql.expression.function.scalar.datetime.DateTimeProcessor.DateTimeExtractor;
2728
import org.elasticsearch.xpack.sql.expression.function.scalar.math.MathProcessor.MathOperation;
2829
import org.elasticsearch.xpack.sql.expression.function.scalar.math.Round;
2930
import org.elasticsearch.xpack.sql.expression.gen.script.ScriptTemplate;
@@ -1160,4 +1161,38 @@ public void testZonedDateTimeInScripts() {
11601161
+ "\"lang\":\"painless\","
11611162
+ "\"params\":{\"v0\":\"date\",\"v1\":\"P1Y\",\"v2\":\"INTERVAL_YEAR\",\"v3\":\"2019-03-11T12:34:56.000Z\"}},"));
11621163
}
1164+
1165+
public void testChronoFieldBasedDateTimeFunctionsWithMathIntervalAndGroupBy() {
1166+
DateTimeExtractor randomFunction = randomValueOtherThan(DateTimeExtractor.YEAR, () -> randomFrom(DateTimeExtractor.values()));
1167+
PhysicalPlan p = optimizeAndPlan(
1168+
"SELECT "
1169+
+ randomFunction.name()
1170+
+ "(date + INTERVAL 1 YEAR) FROM test GROUP BY " + randomFunction.name() + "(date + INTERVAL 1 YEAR)");
1171+
assertEquals(EsQueryExec.class, p.getClass());
1172+
EsQueryExec eqe = (EsQueryExec) p;
1173+
assertThat(eqe.queryContainer().toString().replaceAll("\\s+", ""), containsString(
1174+
"{\"terms\":{\"script\":{\"source\":\"InternalSqlScriptUtils.dateTimeChrono("
1175+
+ "InternalSqlScriptUtils.add(InternalSqlScriptUtils.docValue(doc,params.v0),"
1176+
+ "InternalSqlScriptUtils.intervalYearMonth(params.v1,params.v2)),params.v3,params.v4)\","
1177+
+ "\"lang\":\"painless\",\"params\":{\"v0\":\"date\",\"v1\":\"P1Y\",\"v2\":\"INTERVAL_YEAR\","
1178+
+ "\"v3\":\"Z\",\"v4\":\"" + randomFunction.chronoField().name() + "\"}},\"missing_bucket\":true,"
1179+
+ "\"value_type\":\"long\",\"order\":\"asc\"}}}]}}}}"));
1180+
}
1181+
1182+
public void testDateTimeFunctionsWithMathIntervalAndGroupBy() {
1183+
String[] functions = new String[] {"DAY_NAME", "MONTH_NAME", "DAY_OF_WEEK", "WEEK_OF_YEAR", "QUARTER"};
1184+
String[] scriptMethods = new String[] {"dayName", "monthName", "dayOfWeek", "weekOfYear", "quarter"};
1185+
int pos = randomIntBetween(0, functions.length - 1);
1186+
PhysicalPlan p = optimizeAndPlan(
1187+
"SELECT "
1188+
+ functions[pos]
1189+
+ "(date + INTERVAL 1 YEAR) FROM test GROUP BY " + functions[pos] + "(date + INTERVAL 1 YEAR)");
1190+
assertEquals(EsQueryExec.class, p.getClass());
1191+
EsQueryExec eqe = (EsQueryExec) p;
1192+
assertThat(eqe.queryContainer().toString().replaceAll("\\s+", ""), containsString(
1193+
"{\"terms\":{\"script\":{\"source\":\"InternalSqlScriptUtils." + scriptMethods[pos]
1194+
+ "(InternalSqlScriptUtils.add(InternalSqlScriptUtils.docValue(doc,params.v0),"
1195+
+ "InternalSqlScriptUtils.intervalYearMonth(params.v1,params.v2)),params.v3)\",\"lang\":\"painless\","
1196+
+ "\"params\":{\"v0\":\"date\",\"v1\":\"P1Y\",\"v2\":\"INTERVAL_YEAR\",\"v3\":\"Z\"}},\"missing_bucket\":true,"));
1197+
}
11631198
}

0 commit comments

Comments
 (0)