Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

SQL: rewrite ROUND and TRUNCATE functions with a different optional parameter handling method #40242

Merged
merged 8 commits into from
Mar 21, 2019
17 changes: 17 additions & 0 deletions x-pack/plugin/sql/qa/src/main/resources/agg.csv-spec
Original file line number Diff line number Diff line change
Expand Up @@ -200,6 +200,23 @@ ROUND(SQRT(CAST(EXP(languages) AS SMALLINT)),2):d| COUNT(*):l
null |10
;

groupByRoundWithTwoParams
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can't this be tested in QueryTranslatorTests?

SELECT ROUND(YEAR("birth_date"), -2) FROM test_emp GROUP BY ROUND(YEAR("birth_date"), -2);

ROUND(YEAR("birth_date"), -2)
-----------------------------
null
2000
;

groupByTruncateWithTwoParams
SELECT TRUNCATE(YEAR("birth_date"), -2) FROM test_emp GROUP BY TRUNCATE(YEAR("birth_date"), -2);

TRUNCATE(YEAR("birth_date"), -2)
--------------------------------
null
1900
;

//
// Grouping functions
Expand Down
25 changes: 25 additions & 0 deletions x-pack/plugin/sql/qa/src/main/resources/agg.sql-spec
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,31 @@ SELECT emp_no * 2 AS e FROM test_emp GROUP BY e ORDER BY e;
groupByModScalar
SELECT (emp_no % 3) + 1 AS e FROM test_emp GROUP BY e ORDER BY e;

// group by nested functions with no alias
//https://github.com/elastic/elasticsearch/issues/40239
groupByTruncate-Ignore
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);
//https://github.com/elastic/elasticsearch/issues/40239
groupByRound-Ignore
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);
groupByAtan2
SELECT ATAN2(YEAR("birth_date"), 5) FROM test_emp GROUP BY ATAN2(YEAR("birth_date"), 5) ORDER BY ATAN2(YEAR("birth_date"), 5);
groupByPower
SELECT POWER(YEAR("birth_date"), 2) FROM test_emp GROUP BY POWER(YEAR("birth_date"), 2) ORDER BY POWER(YEAR("birth_date"), 2);
//https://github.com/elastic/elasticsearch/issues/40239
groupByPowerWithCast-Ignore
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);
groupByConcat
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;
groupByLocateWithTwoParams
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"));
groupByLocateWithThreeParams
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);
groupByRoundAndTruncateWithTwoParams
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;
groupByRoundAndTruncateWithOneParam
SELECT ROUND(SIN(TRUNCATE("languages"))) FROM "test_emp" GROUP BY ROUND(SIN(TRUNCATE("languages"))) ORDER BY ROUND(SIN(TRUNCATE("languages"))) LIMIT 5;

// multiple group by
groupByMultiOnText
SELECT gender g, languages l FROM "test_emp" GROUP BY gender, languages ORDER BY gender ASC, languages ASC;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
import org.elasticsearch.xpack.sql.expression.function.scalar.datetime.NonIsoDateTimeProcessor;
import org.elasticsearch.xpack.sql.expression.function.scalar.datetime.QuarterProcessor;
import org.elasticsearch.xpack.sql.expression.function.scalar.math.BinaryMathProcessor;
import org.elasticsearch.xpack.sql.expression.function.scalar.math.BinaryOptionalMathProcessor;
import org.elasticsearch.xpack.sql.expression.function.scalar.math.MathProcessor;
import org.elasticsearch.xpack.sql.expression.function.scalar.string.BinaryStringNumericProcessor;
import org.elasticsearch.xpack.sql.expression.function.scalar.string.BinaryStringStringProcessor;
Expand Down Expand Up @@ -68,7 +69,6 @@ public static List<NamedWriteableRegistry.Entry> getNamedWriteables() {
// arithmetic
entries.add(new Entry(Processor.class, BinaryArithmeticProcessor.NAME, BinaryArithmeticProcessor::new));
entries.add(new Entry(Processor.class, UnaryArithmeticProcessor.NAME, UnaryArithmeticProcessor::new));
entries.add(new Entry(Processor.class, BinaryMathProcessor.NAME, BinaryMathProcessor::new));
// comparators
entries.add(new Entry(Processor.class, BinaryComparisonProcessor.NAME, BinaryComparisonProcessor::new));
entries.add(new Entry(Processor.class, InProcessor.NAME, InProcessor::new));
Expand All @@ -82,6 +82,8 @@ public static List<NamedWriteableRegistry.Entry> getNamedWriteables() {
entries.add(new Entry(Processor.class, NonIsoDateTimeProcessor.NAME, NonIsoDateTimeProcessor::new));
entries.add(new Entry(Processor.class, QuarterProcessor.NAME, QuarterProcessor::new));
// math
entries.add(new Entry(Processor.class, BinaryMathProcessor.NAME, BinaryMathProcessor::new));
entries.add(new Entry(Processor.class, BinaryOptionalMathProcessor.NAME, BinaryOptionalMathProcessor::new));
entries.add(new Entry(Processor.class, MathProcessor.NAME, MathProcessor::new));
// string
entries.add(new Entry(Processor.class, StringProcessor.NAME, StringProcessor::new));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,26 +25,7 @@ public enum BinaryMathOperation implements BiFunction<Number, Number, Number> {

ATAN2((l, r) -> Math.atan2(l.doubleValue(), r.doubleValue())),
MOD(Arithmetics::mod),
POWER((l, r) -> Math.pow(l.doubleValue(), r.doubleValue())),
ROUND((l, r) -> {
if (r instanceof Float || r instanceof Double) {
throw new SqlIllegalArgumentException("An integer number is required; received [{}] as second parameter", r);
}

double tenAtScale = Math.pow(10., r.longValue());
double middleResult = l.doubleValue() * tenAtScale;
int sign = middleResult > 0 ? 1 : -1;
return Math.round(Math.abs(middleResult)) / tenAtScale * sign;
}),
TRUNCATE((l, r) -> {
if (r instanceof Float || r instanceof Double) {
throw new SqlIllegalArgumentException("An integer number is required; received [{}] as second parameter", r);
}

double tenAtScale = Math.pow(10., r.longValue());
double g = l.doubleValue() * tenAtScale;
return (((l.doubleValue() < 0) ? Math.ceil(g) : Math.floor(g)) / tenAtScale);
});
POWER((l, r) -> Math.pow(l.doubleValue(), r.doubleValue()));

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

Expand Down Expand Up @@ -79,7 +60,7 @@ public String getWriteableName() {
@Override
protected void checkParameter(Object param) {
if (!(param instanceof Number)) {
throw new SqlIllegalArgumentException("A number is required; received {}", param);
throw new SqlIllegalArgumentException("A number is required; received [{}]", param);
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,6 @@ public boolean equals(Object obj) {
BinaryNumericFunction other = (BinaryNumericFunction) obj;
return Objects.equals(other.left(), left())
&& Objects.equals(other.right(), right())
&& Objects.equals(other.operation, operation);
&& Objects.equals(other.operation, operation);
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,115 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License;
* you may not use this file except in compliance with the Elastic License.
*/

package org.elasticsearch.xpack.sql.expression.function.scalar.math;

import org.elasticsearch.xpack.sql.execution.search.SqlSourceBuilder;
import org.elasticsearch.xpack.sql.expression.Expression;
import org.elasticsearch.xpack.sql.expression.function.scalar.math.BinaryOptionalMathProcessor.BinaryOptionalMathOperation;
import org.elasticsearch.xpack.sql.expression.gen.pipeline.Pipe;
import org.elasticsearch.xpack.sql.tree.NodeInfo;
import org.elasticsearch.xpack.sql.tree.Source;

import java.util.Arrays;
import java.util.List;
import java.util.Objects;

public class BinaryOptionalMathPipe extends Pipe {

private final Pipe left, right;
private final BinaryOptionalMathOperation operation;

public BinaryOptionalMathPipe(Source source, Expression expression, Pipe left, Pipe right, BinaryOptionalMathOperation operation) {
super(source, expression, right == null ? Arrays.asList(left) : Arrays.asList(left, right));
this.left = left;
this.right = right;
this.operation = operation;
}

@Override
public final Pipe replaceChildren(List<Pipe> newChildren) {
int childrenSize = newChildren.size();
if (childrenSize > 2 || childrenSize < 1) {
throw new IllegalArgumentException("expected [1 or 2] children but received [" + newChildren.size() + "]");
}
return replaceChildren(newChildren.get(0), childrenSize == 1 ? null : newChildren.get(1));
}

@Override
public final Pipe resolveAttributes(AttributeResolver resolver) {
Pipe newLeft = left.resolveAttributes(resolver);
Pipe newRight = right == null ? right : right.resolveAttributes(resolver);
if (newLeft == left && newRight == right) {
return this;
}
return replaceChildren(newLeft, newRight);
}

@Override
public boolean supportedByAggsOnlyQuery() {
return right == null ? left.supportedByAggsOnlyQuery() : left.supportedByAggsOnlyQuery() || right.supportedByAggsOnlyQuery();
}

@Override
public boolean resolved() {
return left.resolved() && (right == null || right.resolved());
}

protected Pipe replaceChildren(Pipe newLeft, Pipe newRight) {
return new BinaryOptionalMathPipe(source(), expression(), newLeft, newRight, operation);
}

@Override
public final void collectFields(SqlSourceBuilder sourceBuilder) {
left.collectFields(sourceBuilder);
if (right != null) {
right.collectFields(sourceBuilder);
}
}

@Override
protected NodeInfo<BinaryOptionalMathPipe> info() {
return NodeInfo.create(this, BinaryOptionalMathPipe::new, expression(), left, right, operation);
}

@Override
public BinaryOptionalMathProcessor asProcessor() {
return new BinaryOptionalMathProcessor(left.asProcessor(), right == null ? null : right.asProcessor(), operation);
}

public Pipe right() {
return right;
}

public Pipe left() {
return left;
}

public BinaryOptionalMathOperation operation() {
return operation;
}

@Override
public int hashCode() {
return Objects.hash(left, right, operation);
}

@Override
public boolean equals(Object obj) {
if (this == obj) {
return true;
}

if (obj == null || getClass() != obj.getClass()) {
return false;
}

BinaryOptionalMathPipe other = (BinaryOptionalMathPipe) obj;
return Objects.equals(left, other.left)
&& Objects.equals(right, other.right)
&& Objects.equals(operation, other.operation);
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,154 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License;
* you may not use this file except in compliance with the Elastic License.
*/

package org.elasticsearch.xpack.sql.expression.function.scalar.math;

import org.elasticsearch.common.io.stream.StreamInput;
import org.elasticsearch.common.io.stream.StreamOutput;
import org.elasticsearch.xpack.sql.SqlIllegalArgumentException;
import org.elasticsearch.xpack.sql.expression.gen.processor.Processor;

import java.io.IOException;
import java.util.Objects;
import java.util.function.BiFunction;

/**
* Processor for binary mathematical operations that can have the second parameter optional.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe rephrase: ... that have a second optional parameter.

*/
public class BinaryOptionalMathProcessor implements Processor {

public enum BinaryOptionalMathOperation implements BiFunction<Number, Number, Number> {

ROUND((l, r) -> {
double tenAtScale = Math.pow(10., r.longValue());
double middleResult = l.doubleValue() * tenAtScale;
int sign = middleResult > 0 ? 1 : -1;
return Math.round(Math.abs(middleResult)) / tenAtScale * sign;
}),
TRUNCATE((l, r) -> {
double tenAtScale = Math.pow(10., r.longValue());
double g = l.doubleValue() * tenAtScale;
return (((l.doubleValue() < 0) ? Math.ceil(g) : Math.floor(g)) / tenAtScale);
});

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

BinaryOptionalMathOperation(BiFunction<Number, Number, Number> process) {
this.process = process;
}

@Override
public final Number apply(Number left, Number right) {
if (left == null) {
return null;
}
if (!(left instanceof Number)) {
throw new SqlIllegalArgumentException("A number is required; received [{}]", left);
}

if (right != null) {
if (!(right instanceof Number)) {
throw new SqlIllegalArgumentException("A number is required; received [{}]", right);
}
if (right instanceof Float || right instanceof Double) {
throw new SqlIllegalArgumentException("An integer number is required; received [{}] as second parameter", right);
}
} else {
right = 0;
}

return process.apply(left, right);
}
}

private final Processor left, right;
private final BinaryOptionalMathOperation operation;
public static final String NAME = "mob";

public BinaryOptionalMathProcessor(Processor left, Processor right, BinaryOptionalMathOperation operation) {
this.left = left;
this.right = right;
this.operation = operation;
}

public BinaryOptionalMathProcessor(StreamInput in) throws IOException {
left = in.readNamedWriteable(Processor.class);
right = in.readOptionalNamedWriteable(Processor.class);
operation = in.readEnum(BinaryOptionalMathOperation.class);
}

@Override
public final void writeTo(StreamOutput out) throws IOException {
out.writeNamedWriteable(left);
out.writeOptionalNamedWriteable(right);
out.writeEnum(operation);
}

@Override
public Object process(Object input) {
return doProcess(left().process(input), right() == null ? null : right().process(input));
}

public Number doProcess(Object left, Object right) {
if (left == null) {
return null;
}
if (!(left instanceof Number)) {
throw new SqlIllegalArgumentException("A number is required; received [{}]", left);
}

if (right != null) {
if (!(right instanceof Number)) {
throw new SqlIllegalArgumentException("A number is required; received [{}]", right);
}
if (right instanceof Float || right instanceof Double) {
throw new SqlIllegalArgumentException("An integer number is required; received [{}] as second parameter", right);
}
} else {
right = 0;
}

return operation().apply((Number) left, (Number) right);
}

@Override
public boolean equals(Object obj) {
if (this == obj) {
return true;
}

if (obj == null || getClass() != obj.getClass()) {
return false;
}

BinaryOptionalMathProcessor other = (BinaryOptionalMathProcessor) obj;
return Objects.equals(left(), other.left())
&& Objects.equals(right(), other.right())
&& Objects.equals(operation(), other.operation());
}

@Override
public int hashCode() {
return Objects.hash(left(), right(), operation());
}

public Processor left() {
return left;
}

public Processor right() {
return right;
}

public BinaryOptionalMathOperation operation() {
return operation;
}

@Override
public String getWriteableName() {
return NAME;
}
}
Loading