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

Handle NULL arithmetic operations with INTERVALs #49633

Merged
merged 5 commits into from
Dec 2, 2019
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion docs/reference/sql/functions/date-time.asciidoc
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ s|Description

==== Operators

Basic arithmetic operators (`+`, `-`, etc) support date/time parameters as indicated below:
Basic arithmetic operators (`+`, `-`, `*`) support date/time parameters as indicated below:

[source, sql]
--------------------------------------------------
Expand Down
36 changes: 36 additions & 0 deletions x-pack/plugin/sql/qa/src/main/resources/datetime-interval.csv-spec
Original file line number Diff line number Diff line change
Expand Up @@ -142,6 +142,15 @@ INTERVAL 1 DAY + INTERVAL 53 MINUTES
+1 00:53:00.0
;

intervalPlusNull
schema::result:string
SELECT INTERVAL 1 DAY + null AS result;

result
------------------------------------
null
;

datePlusIntervalInline
SELECT CAST('1969-05-13T12:34:56' AS DATETIME) + INTERVAL 49 YEARS AS result;

Expand All @@ -167,6 +176,24 @@ SELECT INTERVAL '1' DAY - INTERVAL '2' HOURS AS result;
+0 22:00:00.0
;

intervalMinusNull
schema::result:string
SELECT INTERVAL '1' DAY - null AS result;

result
---------------
null
;

nullMinusInterval
schema::result:string
SELECT null - INTERVAL '1' DAY AS result;

result
---------------
null
;


intervalYearMultiply
SELECT -2 * INTERVAL '3' YEARS AS result;
Expand All @@ -184,6 +211,15 @@ SELECT -2 * INTERVAL '1 23:45' DAY TO MINUTES AS result;
-3 23:30:00.0
;

intervalNullMultiply
schema::result:string
SELECT null * INTERVAL '1 23:45' DAY TO MINUTES AS result;

result
---------------
null
;

intervalHoursMultiply
SELECT 4 * -INTERVAL '2' HOURS AS result1, -5 * -INTERVAL '3' HOURS AS result2;
result1 | result2
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,17 +5,16 @@
*/
package org.elasticsearch.xpack.sql.expression;

import org.elasticsearch.xpack.sql.expression.Expression.TypeResolution;
import org.elasticsearch.xpack.sql.expression.Expressions.ParamOrdinal;
import org.elasticsearch.xpack.sql.type.DataType;
import org.elasticsearch.xpack.sql.type.DataTypes;
import org.elasticsearch.xpack.sql.type.EsField;

import java.util.Locale;
import java.util.StringJoiner;
import java.util.function.Predicate;

import static org.elasticsearch.common.logging.LoggerMessageFormat.format;
import static org.elasticsearch.xpack.sql.expression.Expression.TypeResolution;
import static org.elasticsearch.xpack.sql.expression.Expressions.ParamOrdinal;
import static org.elasticsearch.xpack.sql.expression.Expressions.name;
import static org.elasticsearch.xpack.sql.type.DataType.BOOLEAN;

Expand Down Expand Up @@ -119,7 +118,7 @@ public static TypeResolution isType(Expression e,
String operationName,
ParamOrdinal paramOrd,
String... acceptedTypes) {
return predicate.test(e.dataType()) || DataTypes.isNull(e.dataType())?
return predicate.test(e.dataType()) || e.dataType().isNull() ?
TypeResolution.TYPE_RESOLVED :
new TypeResolution(format(null, "{}argument of [{}] must be [{}], found value [{}] type [{}]",
paramOrd == null || paramOrd == ParamOrdinal.DEFAULT ? "" : paramOrd.name().toLowerCase(Locale.ROOT) + " ",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@
import org.elasticsearch.xpack.sql.tree.Source;
import org.elasticsearch.xpack.sql.type.DataType;
import org.elasticsearch.xpack.sql.type.DataTypeConversion;
import org.elasticsearch.xpack.sql.type.DataTypes;

import java.util.Objects;

Expand Down Expand Up @@ -64,7 +63,7 @@ public Object fold() {

@Override
public Nullability nullable() {
if (DataTypes.isNull(from())) {
if (from().isNull()) {
return Nullability.TRUE;
}
return field().nullable();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ protected NodeInfo<? extends Expression> info() {
protected TypeResolution resolveType() {
DataType expectedResultDataType = null;
for (IfConditional ifConditional : conditions) {
if (DataTypes.isNull(ifConditional.result().dataType()) == false) {
if (ifConditional.result().dataType().isNull() == false) {
expectedResultDataType = ifConditional.result().dataType();
break;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,10 +12,9 @@
import org.elasticsearch.xpack.sql.expression.gen.script.Scripts;
import org.elasticsearch.xpack.sql.expression.predicate.Negatable;
import org.elasticsearch.xpack.sql.expression.predicate.nulls.CheckNullProcessor.CheckNullOperation;
import org.elasticsearch.xpack.sql.tree.Source;
import org.elasticsearch.xpack.sql.tree.NodeInfo;
import org.elasticsearch.xpack.sql.tree.Source;
import org.elasticsearch.xpack.sql.type.DataType;
import org.elasticsearch.xpack.sql.type.DataTypes;

public class IsNotNull extends UnaryScalarFunction implements Negatable<UnaryScalarFunction> {

Expand All @@ -35,7 +34,7 @@ protected IsNotNull replaceChild(Expression newChild) {

@Override
public Object fold() {
return field().fold() != null && !DataTypes.isNull(field().dataType());
return field().fold() != null && !field().dataType().isNull();
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,10 +12,9 @@
import org.elasticsearch.xpack.sql.expression.gen.script.Scripts;
import org.elasticsearch.xpack.sql.expression.predicate.Negatable;
import org.elasticsearch.xpack.sql.expression.predicate.nulls.CheckNullProcessor.CheckNullOperation;
import org.elasticsearch.xpack.sql.tree.Source;
import org.elasticsearch.xpack.sql.tree.NodeInfo;
import org.elasticsearch.xpack.sql.tree.Source;
import org.elasticsearch.xpack.sql.type.DataType;
import org.elasticsearch.xpack.sql.type.DataTypes;

public class IsNull extends UnaryScalarFunction implements Negatable<UnaryScalarFunction> {

Expand All @@ -35,7 +34,7 @@ protected IsNull replaceChild(Expression newChild) {

@Override
public Object fold() {
return field().fold() == null || DataTypes.isNull(field().dataType());
return field().fold() == null || field().dataType().isNull();
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ protected TypeResolution resolveWithIntervals() {
DataType l = left().dataType();
DataType r = right().dataType();

if (!(r.isDateOrTimeBased() || r.isInterval())|| !(l.isDateOrTimeBased() || l.isInterval())) {
if (!(r.isDateOrTimeBased() || r.isInterval() || r.isNull())|| !(l.isDateOrTimeBased() || l.isInterval() || l.isNull())) {
return new TypeResolution(format(null, "[{}] has arguments with incompatible types [{}] and [{}]", symbol(), l, r));
}
return TypeResolution.TYPE_RESOLVED;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,14 +34,14 @@ protected TypeResolution resolveType() {
DataType r = right().dataType();

// 1. both are numbers
if (l.isNumeric() && r.isNumeric()) {
if (l.isNullOrNumeric() && r.isNullOrNumeric()) {
return TypeResolution.TYPE_RESOLVED;
}

if (l.isInterval() && r.isInteger()) {
if (l.isNullOrInterval() && (r.isInteger() || r.isNull())) {
dataType = l;
return TypeResolution.TYPE_RESOLVED;
} else if (r.isInterval() && l.isInteger()) {
} else if (r.isNullOrInterval() && (l.isInteger() || l.isNull())) {
dataType = r;
return TypeResolution.TYPE_RESOLVED;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@
import org.elasticsearch.xpack.sql.expression.Expression;
import org.elasticsearch.xpack.sql.expression.Foldables;
import org.elasticsearch.xpack.sql.tree.Source;
import org.elasticsearch.xpack.sql.type.DataTypes;

import java.util.Collections;
import java.util.LinkedHashSet;
Expand All @@ -27,7 +26,7 @@ public class TermsQuery extends LeafQuery {
public TermsQuery(Source source, String term, List<Expression> values) {
super(source);
this.term = term;
values.removeIf(e -> DataTypes.isNull(e.dataType()));
values.removeIf(e -> e.dataType().isNull());
if (values.isEmpty()) {
this.values = Collections.emptySet();
} else {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -247,6 +247,18 @@ public boolean isSigned() {
return isNumeric();
}

public boolean isNull() {
return this == NULL;
}

public boolean isNullOrNumeric() {
return isNull() || isNumeric();
}

public boolean isNullOrInterval() {
return isNull() || isInterval();
}

public boolean isString() {
return this == KEYWORD || this == TEXT;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,10 +45,10 @@ public static DataType commonType(DataType left, DataType right) {
if (left == right) {
return left;
}
if (DataTypes.isNull(left)) {
if (left.isNull()) {
return right;
}
if (DataTypes.isNull(right)) {
if (right.isNull()) {
return left;
}
if (left.isString() && right.isString()) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,10 +31,6 @@ public final class DataTypes {

private DataTypes() {}

public static boolean isNull(DataType from) {
return from == NULL;
}

public static boolean isUnsupported(DataType from) {
return from == UNSUPPORTED;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -227,6 +227,45 @@ public void testMulNumberInterval() {
Period p = interval.interval();
assertEquals(Period.ofYears(2).negated(), p);
}

public void testMulNullInterval() {
Literal literal = interval(Period.ofMonths(1), INTERVAL_MONTH);
Mul result = new Mul(EMPTY, L(null), literal);
assertTrue(result.foldable());
assertNull(result.fold());
assertEquals(INTERVAL_MONTH, result.dataType());

result = new Mul(EMPTY, literal, L(null));
assertTrue(result.foldable());
assertNull(result.fold());
assertEquals(INTERVAL_MONTH, result.dataType());
}

public void testAddNullInterval() {
Literal literal = interval(Period.ofMonths(1), INTERVAL_MONTH);
Add result = new Add(EMPTY, L(null), literal);
assertTrue(result.foldable());
assertNull(result.fold());
assertEquals(INTERVAL_MONTH, result.dataType());

result = new Add(EMPTY, literal, L(null));
assertTrue(result.foldable());
assertNull(result.fold());
assertEquals(INTERVAL_MONTH, result.dataType());
}

public void testSubNullInterval() {
Literal literal = interval(Period.ofMonths(1), INTERVAL_MONTH);
Sub result = new Sub(EMPTY, L(null), literal);
assertTrue(result.foldable());
assertNull(result.fold());
assertEquals(INTERVAL_MONTH, result.dataType());

result = new Sub(EMPTY, literal, L(null));
assertTrue(result.foldable());
assertNull(result.fold());
assertEquals(INTERVAL_MONTH, result.dataType());
}

@SuppressWarnings("unchecked")
private static <T> T add(Object l, Object r) {
Expand Down