Skip to content

Commit ac8ef99

Browse files
astefanastefan
committed
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. (cherry picked from commit 3e314f8)
1 parent 306af42 commit ac8ef99

File tree

16 files changed

+546
-53
lines changed

16 files changed

+546
-53
lines changed

‎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)