Skip to content

Commit 3e314f8

Browse files
astefanastefan
authored
SQL: rewrite ROUND and TRUNCATE functions with a different optional parameter handling method (#40242)
* Rewrite Round and Truncate functions to have a slightly different approach to handling the optional parameter in the constructor. Until now the optional parameter was considered 0 if the value was missing and the constructor was filling in this value. The current solution is to have the optional parameter as null right until the actual calculation is done.
1 parent 3d90fbc commit 3e314f8

16 files changed

Lines changed: 546 additions & 53 deletions

File tree

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

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -200,6 +200,23 @@ ROUND(SQRT(CAST(EXP(languages) AS SMALLINT)),2):d| COUNT(*):l
200200
null |10
201201
;
202202

203+
groupByRoundWithTwoParams
204+
SELECT ROUND(YEAR("birth_date"), -2) FROM test_emp GROUP BY ROUND(YEAR("birth_date"), -2);
205+
206+
ROUND(YEAR("birth_date"), -2)
207+
-----------------------------
208+
null
209+
2000
210+
;
211+
212+
groupByTruncateWithTwoParams
213+
SELECT TRUNCATE(YEAR("birth_date"), -2) FROM test_emp GROUP BY TRUNCATE(YEAR("birth_date"), -2);
214+
215+
TRUNCATE(YEAR("birth_date"), -2)
216+
--------------------------------
217+
null
218+
1900
219+
;
203220

204221
//
205222
// Grouping functions

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

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,31 @@ SELECT emp_no * 2 AS e FROM test_emp GROUP BY e ORDER BY e;
5050
groupByModScalar
5151
SELECT (emp_no % 3) + 1 AS e FROM test_emp GROUP BY e ORDER BY e;
5252

53+
// group by nested functions with no alias
54+
//https://github.com/elastic/elasticsearch/issues/40239
55+
groupByTruncate-Ignore
56+
SELECT CAST(TRUNCATE(EXTRACT(YEAR FROM "birth_date")) AS INTEGER) FROM test_emp GROUP BY CAST(TRUNCATE(EXTRACT(YEAR FROM "birth_date")) AS INTEGER) ORDER BY CAST(TRUNCATE(EXTRACT(YEAR FROM "birth_date")) AS INTEGER);
57+
//https://github.com/elastic/elasticsearch/issues/40239
58+
groupByRound-Ignore
59+
SELECT CAST(ROUND(EXTRACT(YEAR FROM "birth_date")) AS INTEGER) FROM test_emp GROUP BY CAST(ROUND(EXTRACT(YEAR FROM "birth_date")) AS INTEGER) ORDER BY CAST(ROUND(EXTRACT(YEAR FROM "birth_date")) AS INTEGER);
60+
groupByAtan2
61+
SELECT ATAN2(YEAR("birth_date"), 5) FROM test_emp GROUP BY ATAN2(YEAR("birth_date"), 5) ORDER BY ATAN2(YEAR("birth_date"), 5);
62+
groupByPower
63+
SELECT POWER(YEAR("birth_date"), 2) FROM test_emp GROUP BY POWER(YEAR("birth_date"), 2) ORDER BY POWER(YEAR("birth_date"), 2);
64+
//https://github.com/elastic/elasticsearch/issues/40239
65+
groupByPowerWithCast-Ignore
66+
SELECT CAST(POWER(YEAR("birth_date"), 2) AS DOUBLE) FROM test_emp GROUP BY CAST(POWER(YEAR("birth_date"), 2) AS DOUBLE) ORDER BY CAST(POWER(YEAR("birth_date"), 2) AS DOUBLE);
67+
groupByConcat
68+
SELECT LEFT(CONCAT("first_name", "last_name"), 3) FROM test_emp GROUP BY LEFT(CONCAT("first_name", "last_name"), 3) ORDER BY LEFT(CONCAT("first_name", "last_name"), 3) LIMIT 15;
69+
groupByLocateWithTwoParams
70+
SELECT LOCATE('a', CONCAT("first_name", "last_name")) FROM test_emp GROUP BY LOCATE('a', CONCAT("first_name", "last_name")) ORDER BY LOCATE('a', CONCAT("first_name", "last_name"));
71+
groupByLocateWithThreeParams
72+
SELECT LOCATE('a', CONCAT("first_name", "last_name"), 3) FROM test_emp GROUP BY LOCATE('a', CONCAT("first_name", "last_name"), 3) ORDER BY LOCATE('a', CONCAT("first_name", "last_name"), 3);
73+
groupByRoundAndTruncateWithTwoParams
74+
SELECT ROUND(SIN(TRUNCATE("salary", 2)), 2) FROM "test_emp" GROUP BY ROUND(SIN(TRUNCATE("salary", 2)), 2) ORDER BY ROUND(SIN(TRUNCATE("salary", 2)), 2) LIMIT 5;
75+
groupByRoundAndTruncateWithOneParam
76+
SELECT ROUND(SIN(TRUNCATE("languages"))) FROM "test_emp" GROUP BY ROUND(SIN(TRUNCATE("languages"))) ORDER BY ROUND(SIN(TRUNCATE("languages"))) LIMIT 5;
77+
5378
// multiple group by
5479
groupByMultiOnText
5580
SELECT gender g, languages l FROM "test_emp" GROUP BY gender, languages ORDER BY gender ASC, languages ASC;

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

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212
import org.elasticsearch.xpack.sql.expression.function.scalar.datetime.NonIsoDateTimeProcessor;
1313
import org.elasticsearch.xpack.sql.expression.function.scalar.datetime.QuarterProcessor;
1414
import org.elasticsearch.xpack.sql.expression.function.scalar.math.BinaryMathProcessor;
15+
import org.elasticsearch.xpack.sql.expression.function.scalar.math.BinaryOptionalMathProcessor;
1516
import org.elasticsearch.xpack.sql.expression.function.scalar.math.MathProcessor;
1617
import org.elasticsearch.xpack.sql.expression.function.scalar.string.BinaryStringNumericProcessor;
1718
import org.elasticsearch.xpack.sql.expression.function.scalar.string.BinaryStringStringProcessor;
@@ -68,7 +69,6 @@ public static List<NamedWriteableRegistry.Entry> getNamedWriteables() {
6869
// arithmetic
6970
entries.add(new Entry(Processor.class, BinaryArithmeticProcessor.NAME, BinaryArithmeticProcessor::new));
7071
entries.add(new Entry(Processor.class, UnaryArithmeticProcessor.NAME, UnaryArithmeticProcessor::new));
71-
entries.add(new Entry(Processor.class, BinaryMathProcessor.NAME, BinaryMathProcessor::new));
7272
// comparators
7373
entries.add(new Entry(Processor.class, BinaryComparisonProcessor.NAME, BinaryComparisonProcessor::new));
7474
entries.add(new Entry(Processor.class, InProcessor.NAME, InProcessor::new));
@@ -82,6 +82,8 @@ public static List<NamedWriteableRegistry.Entry> getNamedWriteables() {
8282
entries.add(new Entry(Processor.class, NonIsoDateTimeProcessor.NAME, NonIsoDateTimeProcessor::new));
8383
entries.add(new Entry(Processor.class, QuarterProcessor.NAME, QuarterProcessor::new));
8484
// math
85+
entries.add(new Entry(Processor.class, BinaryMathProcessor.NAME, BinaryMathProcessor::new));
86+
entries.add(new Entry(Processor.class, BinaryOptionalMathProcessor.NAME, BinaryOptionalMathProcessor::new));
8587
entries.add(new Entry(Processor.class, MathProcessor.NAME, MathProcessor::new));
8688
// string
8789
entries.add(new Entry(Processor.class, StringProcessor.NAME, StringProcessor::new));

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

Lines changed: 2 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -25,26 +25,7 @@ public enum BinaryMathOperation implements BiFunction<Number, Number, Number> {
2525

2626
ATAN2((l, r) -> Math.atan2(l.doubleValue(), r.doubleValue())),
2727
MOD(Arithmetics::mod),
28-
POWER((l, r) -> Math.pow(l.doubleValue(), r.doubleValue())),
29-
ROUND((l, r) -> {
30-
if (r instanceof Float || r instanceof Double) {
31-
throw new SqlIllegalArgumentException("An integer number is required; received [{}] as second parameter", r);
32-
}
33-
34-
double tenAtScale = Math.pow(10., r.longValue());
35-
double middleResult = l.doubleValue() * tenAtScale;
36-
int sign = middleResult > 0 ? 1 : -1;
37-
return Math.round(Math.abs(middleResult)) / tenAtScale * sign;
38-
}),
39-
TRUNCATE((l, r) -> {
40-
if (r instanceof Float || r instanceof Double) {
41-
throw new SqlIllegalArgumentException("An integer number is required; received [{}] as second parameter", r);
42-
}
43-
44-
double tenAtScale = Math.pow(10., r.longValue());
45-
double g = l.doubleValue() * tenAtScale;
46-
return (((l.doubleValue() < 0) ? Math.ceil(g) : Math.floor(g)) / tenAtScale);
47-
});
28+
POWER((l, r) -> Math.pow(l.doubleValue(), r.doubleValue()));
4829

4930
private final BiFunction<Number, Number, Number> process;
5031

@@ -79,7 +60,7 @@ public String getWriteableName() {
7960
@Override
8061
protected void checkParameter(Object param) {
8162
if (!(param instanceof Number)) {
82-
throw new SqlIllegalArgumentException("A number is required; received {}", param);
63+
throw new SqlIllegalArgumentException("A number is required; received [{}]", param);
8364
}
8465
}
8566
}

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -69,6 +69,6 @@ public boolean equals(Object obj) {
6969
BinaryNumericFunction other = (BinaryNumericFunction) obj;
7070
return Objects.equals(other.left(), left())
7171
&& Objects.equals(other.right(), right())
72-
&& Objects.equals(other.operation, operation);
72+
&& Objects.equals(other.operation, operation);
7373
}
7474
}

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

Lines changed: 115 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,115 @@
1+
/*
2+
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
3+
* or more contributor license agreements. Licensed under the Elastic License;
4+
* you may not use this file except in compliance with the Elastic License.
5+
*/
6+
7+
package org.elasticsearch.xpack.sql.expression.function.scalar.math;
8+
9+
import org.elasticsearch.xpack.sql.execution.search.SqlSourceBuilder;
10+
import org.elasticsearch.xpack.sql.expression.Expression;
11+
import org.elasticsearch.xpack.sql.expression.function.scalar.math.BinaryOptionalMathProcessor.BinaryOptionalMathOperation;
12+
import org.elasticsearch.xpack.sql.expression.gen.pipeline.Pipe;
13+
import org.elasticsearch.xpack.sql.tree.NodeInfo;
14+
import org.elasticsearch.xpack.sql.tree.Source;
15+
16+
import java.util.Arrays;
17+
import java.util.List;
18+
import java.util.Objects;
19+
20+
public class BinaryOptionalMathPipe extends Pipe {
21+
22+
private final Pipe left, right;
23+
private final BinaryOptionalMathOperation operation;
24+
25+
public BinaryOptionalMathPipe(Source source, Expression expression, Pipe left, Pipe right, BinaryOptionalMathOperation operation) {
26+
super(source, expression, right == null ? Arrays.asList(left) : Arrays.asList(left, right));
27+
this.left = left;
28+
this.right = right;
29+
this.operation = operation;
30+
}
31+
32+
@Override
33+
public final Pipe replaceChildren(List<Pipe> newChildren) {
34+
int childrenSize = newChildren.size();
35+
if (childrenSize > 2 || childrenSize < 1) {
36+
throw new IllegalArgumentException("expected [1 or 2] children but received [" + newChildren.size() + "]");
37+
}
38+
return replaceChildren(newChildren.get(0), childrenSize == 1 ? null : newChildren.get(1));
39+
}
40+
41+
@Override
42+
public final Pipe resolveAttributes(AttributeResolver resolver) {
43+
Pipe newLeft = left.resolveAttributes(resolver);
44+
Pipe newRight = right == null ? right : right.resolveAttributes(resolver);
45+
if (newLeft == left && newRight == right) {
46+
return this;
47+
}
48+
return replaceChildren(newLeft, newRight);
49+
}
50+
51+
@Override
52+
public boolean supportedByAggsOnlyQuery() {
53+
return right == null ? left.supportedByAggsOnlyQuery() : left.supportedByAggsOnlyQuery() || right.supportedByAggsOnlyQuery();
54+
}
55+
56+
@Override
57+
public boolean resolved() {
58+
return left.resolved() && (right == null || right.resolved());
59+
}
60+
61+
protected Pipe replaceChildren(Pipe newLeft, Pipe newRight) {
62+
return new BinaryOptionalMathPipe(source(), expression(), newLeft, newRight, operation);
63+
}
64+
65+
@Override
66+
public final void collectFields(SqlSourceBuilder sourceBuilder) {
67+
left.collectFields(sourceBuilder);
68+
if (right != null) {
69+
right.collectFields(sourceBuilder);
70+
}
71+
}
72+
73+
@Override
74+
protected NodeInfo<BinaryOptionalMathPipe> info() {
75+
return NodeInfo.create(this, BinaryOptionalMathPipe::new, expression(), left, right, operation);
76+
}
77+
78+
@Override
79+
public BinaryOptionalMathProcessor asProcessor() {
80+
return new BinaryOptionalMathProcessor(left.asProcessor(), right == null ? null : right.asProcessor(), operation);
81+
}
82+
83+
public Pipe right() {
84+
return right;
85+
}
86+
87+
public Pipe left() {
88+
return left;
89+
}
90+
91+
public BinaryOptionalMathOperation operation() {
92+
return operation;
93+
}
94+
95+
@Override
96+
public int hashCode() {
97+
return Objects.hash(left, right, operation);
98+
}
99+
100+
@Override
101+
public boolean equals(Object obj) {
102+
if (this == obj) {
103+
return true;
104+
}
105+
106+
if (obj == null || getClass() != obj.getClass()) {
107+
return false;
108+
}
109+
110+
BinaryOptionalMathPipe other = (BinaryOptionalMathPipe) obj;
111+
return Objects.equals(left, other.left)
112+
&& Objects.equals(right, other.right)
113+
&& Objects.equals(operation, other.operation);
114+
}
115+
}

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

Lines changed: 154 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,154 @@
1+
/*
2+
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
3+
* or more contributor license agreements. Licensed under the Elastic License;
4+
* you may not use this file except in compliance with the Elastic License.
5+
*/
6+
7+
package org.elasticsearch.xpack.sql.expression.function.scalar.math;
8+
9+
import org.elasticsearch.common.io.stream.StreamInput;
10+
import org.elasticsearch.common.io.stream.StreamOutput;
11+
import org.elasticsearch.xpack.sql.SqlIllegalArgumentException;
12+
import org.elasticsearch.xpack.sql.expression.gen.processor.Processor;
13+
14+
import java.io.IOException;
15+
import java.util.Objects;
16+
import java.util.function.BiFunction;
17+
18+
/**
19+
* Processor for binary mathematical operations that have a second optional parameter.
20+
*/
21+
public class BinaryOptionalMathProcessor implements Processor {
22+
23+
public enum BinaryOptionalMathOperation implements BiFunction<Number, Number, Number> {
24+
25+
ROUND((l, r) -> {
26+
double tenAtScale = Math.pow(10., r.longValue());
27+
double middleResult = l.doubleValue() * tenAtScale;
28+
int sign = middleResult > 0 ? 1 : -1;
29+
return Math.round(Math.abs(middleResult)) / tenAtScale * sign;
30+
}),
31+
TRUNCATE((l, r) -> {
32+
double tenAtScale = Math.pow(10., r.longValue());
33+
double g = l.doubleValue() * tenAtScale;
34+
return (((l.doubleValue() < 0) ? Math.ceil(g) : Math.floor(g)) / tenAtScale);
35+
});
36+
37+
private final BiFunction<Number, Number, Number> process;
38+
39+
BinaryOptionalMathOperation(BiFunction<Number, Number, Number> process) {
40+
this.process = process;
41+
}
42+
43+
@Override
44+
public final Number apply(Number left, Number right) {
45+
if (left == null) {
46+
return null;
47+
}
48+
if (!(left instanceof Number)) {
49+
throw new SqlIllegalArgumentException("A number is required; received [{}]", left);
50+
}
51+
52+
if (right != null) {
53+
if (!(right instanceof Number)) {
54+
throw new SqlIllegalArgumentException("A number is required; received [{}]", right);
55+
}
56+
if (right instanceof Float || right instanceof Double) {
57+
throw new SqlIllegalArgumentException("An integer number is required; received [{}] as second parameter", right);
58+
}
59+
} else {
60+
right = 0;
61+
}
62+
63+
return process.apply(left, right);
64+
}
65+
}
66+
67+
private final Processor left, right;
68+
private final BinaryOptionalMathOperation operation;
69+
public static final String NAME = "mob";
70+
71+
public BinaryOptionalMathProcessor(Processor left, Processor right, BinaryOptionalMathOperation operation) {
72+
this.left = left;
73+
this.right = right;
74+
this.operation = operation;
75+
}
76+
77+
public BinaryOptionalMathProcessor(StreamInput in) throws IOException {
78+
left = in.readNamedWriteable(Processor.class);
79+
right = in.readOptionalNamedWriteable(Processor.class);
80+
operation = in.readEnum(BinaryOptionalMathOperation.class);
81+
}
82+
83+
@Override
84+
public final void writeTo(StreamOutput out) throws IOException {
85+
out.writeNamedWriteable(left);
86+
out.writeOptionalNamedWriteable(right);
87+
out.writeEnum(operation);
88+
}
89+
90+
@Override
91+
public Object process(Object input) {
92+
return doProcess(left().process(input), right() == null ? null : right().process(input));
93+
}
94+
95+
public Number doProcess(Object left, Object right) {
96+
if (left == null) {
97+
return null;
98+
}
99+
if (!(left instanceof Number)) {
100+
throw new SqlIllegalArgumentException("A number is required; received [{}]", left);
101+
}
102+
103+
if (right != null) {
104+
if (!(right instanceof Number)) {
105+
throw new SqlIllegalArgumentException("A number is required; received [{}]", right);
106+
}
107+
if (right instanceof Float || right instanceof Double) {
108+
throw new SqlIllegalArgumentException("An integer number is required; received [{}] as second parameter", right);
109+
}
110+
} else {
111+
right = 0;
112+
}
113+
114+
return operation().apply((Number) left, (Number) right);
115+
}
116+
117+
@Override
118+
public boolean equals(Object obj) {
119+
if (this == obj) {
120+
return true;
121+
}
122+
123+
if (obj == null || getClass() != obj.getClass()) {
124+
return false;
125+
}
126+
127+
BinaryOptionalMathProcessor other = (BinaryOptionalMathProcessor) obj;
128+
return Objects.equals(left(), other.left())
129+
&& Objects.equals(right(), other.right())
130+
&& Objects.equals(operation(), other.operation());
131+
}
132+
133+
@Override
134+
public int hashCode() {
135+
return Objects.hash(left(), right(), operation());
136+
}
137+
138+
public Processor left() {
139+
return left;
140+
}
141+
142+
public Processor right() {
143+
return right;
144+
}
145+
146+
public BinaryOptionalMathOperation operation() {
147+
return operation;
148+
}
149+
150+
@Override
151+
public String getWriteableName() {
152+
return NAME;
153+
}
154+
}

0 commit comments

Comments
 (0)