Skip to content

Commit

Permalink
SQL: rewrite ROUND and TRUNCATE functions with a different optional p…
Browse files Browse the repository at this point in the history
…arameter 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.
  • Loading branch information
astefan authored Mar 21, 2019
1 parent 3d90fbc commit 3e314f8
Show file tree
Hide file tree
Showing 16 changed files with 546 additions and 53 deletions.
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
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 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

0 comments on commit 3e314f8

Please sign in to comment.