Skip to content

Commit

Permalink
remove implicit casting to intervals for arithmetic operations
Browse files Browse the repository at this point in the history
  • Loading branch information
@fang-xing-esql
fang-xing-esql committed Nov 4, 2024
1 parent dc850d7 commit 8b31b45
Show file tree
Hide file tree
Showing 7 changed files with 68 additions and 364 deletions.
7 changes: 5 additions & 2 deletions x-pack/plugin/esql-core/src/main/java/org/elasticsearch/xpack/esql/core/type/DataType.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,6 @@
import java.util.function.Function;

import static java.util.stream.Collectors.toMap;
import static java.util.stream.Collectors.toUnmodifiableMap;
import static org.elasticsearch.xpack.esql.core.util.PlanStreamInput.readCachedStringWithVersionCheck;
import static org.elasticsearch.xpack.esql.core.util.PlanStreamOutput.writeCachedStringWithVersionCheck;

Expand Down Expand Up @@ -276,7 +275,7 @@ public enum DataType {

private static final Collection<DataType> STRING_TYPES = DataType.types().stream().filter(DataType::isString).toList();

private static final Map<String, DataType> NAME_TO_TYPE = TYPES.stream().collect(toUnmodifiableMap(DataType::typeName, t -> t));
private static final Map<String, DataType> NAME_TO_TYPE;

private static final Map<String, DataType> ES_TO_TYPE;

Expand All @@ -287,6 +286,10 @@ public enum DataType {
map.put("point", DataType.CARTESIAN_POINT);
map.put("shape", DataType.CARTESIAN_SHAPE);
ES_TO_TYPE = Collections.unmodifiableMap(map);
// DATETIME has different esType and typeName, add an entry in NAME_TO_TYPE with date as key
map = TYPES.stream().collect(toMap(DataType::typeName, t -> t));
map.put("date", DataType.DATETIME);
NAME_TO_TYPE = Collections.unmodifiableMap(map);
}

private static final Map<String, DataType> NAME_OR_ALIAS_TO_TYPE;
Expand Down
154 changes: 0 additions & 154 deletions x-pack/plugin/esql/qa/testFixtures/src/main/resources/date.csv-spec
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1380,160 +1380,6 @@ a:integer | year_hired:date
1 | 1991-01-01T00:00:00.000Z
;

implicitCastingToDatePeriodPlusDate
required_capability: implicit_casting_string_literal_to_temporal_amount

ROW x = "2024-01-01"::datetime
| EVAL y = x + "3 DAYS",
z = x - "3 days",
u = "3 DAYS" + x,
v = - "3 DAYS" + x
;

x:datetime |y:datetime |z:datetime |u:datetime |v:datetime
2024-01-01 |2024-01-04 |2023-12-29 |2024-01-04 |2023-12-29
;

implicitCastingToDatePeriodPlusDateInString
required_capability: implicit_casting_string_literal_to_temporal_amount

ROW x = "2024-01-01"::datetime
| EVAL y = "2024-01-01" + "3 DAYS",
z = "2024-01-01" - "3 days",
u = "3 DAYS" + "2024-01-01",
v = - "3 days" + "2024-01-01"
;

x:datetime |y:datetime |z:datetime |u:datetime |v:datetime
2024-01-01 |2024-01-04 |2023-12-29 |2024-01-04 |2023-12-29
;

implicitCastingToTimeDurationPlusDate
required_capability: implicit_casting_string_literal_to_temporal_amount

ROW x = "2024-01-01"::datetime
| EVAL y = x + "3 hours",
z = x - "3 hours",
u = "3 hours" + x,
v = - "3 hours" + x
;

x:datetime |y:datetime |z:datetime |u:datetime |v:datetime
2024-01-01 |2024-01-01T03:00:00.000Z |2023-12-31T21:00:00.000Z |2024-01-01T03:00:00.000Z |2023-12-31T21:00:00.000Z
;

implicitCastingToTimeDurationPlusDateInString
required_capability: implicit_casting_string_literal_to_temporal_amount

ROW x = "2024-01-01"::datetime
| EVAL y = "2024-01-01" + "3 hours",
z = "2024-01-01" - "3 hours",
u = "3 hours" + "2024-01-01",
v = - "3 hours" + "2024-01-01"
;

x:datetime |y:datetime |z:datetime |u:datetime |v:datetime
2024-01-01 |2024-01-01T03:00:00.000Z |2023-12-31T21:00:00.000Z |2024-01-01T03:00:00.000Z |2023-12-31T21:00:00.000Z
;

implicitCastingToDatePeriodFromIndex
required_capability: implicit_casting_string_literal_to_temporal_amount

FROM employees
| WHERE emp_no == 10001
| EVAL x = birth_date + "3 days",
y = "3 days" + birth_date,
z = birth_date - "3 days",
u = - "3 days" + birth_date
| KEEP birth_date, x, y, z, u;

birth_date:datetime |x:datetime |y:datetime |z:datetime |u:datetime
1953-09-02T00:00:00Z |1953-09-05T00:00:00Z |1953-09-05T00:00:00Z |1953-08-30T00:00:00Z |1953-08-30T00:00:00Z
;

implicitCastingToTimeDurationFromIndex
required_capability: implicit_casting_string_literal_to_temporal_amount

FROM employees
| WHERE emp_no == 10001
| EVAL x = birth_date + "3 hours",
y = "3 hours" + birth_date,
z = birth_date - "3 hours",
u = - "3 hours" + birth_date
| KEEP birth_date, x, y, z, u;

birth_date:datetime |x:datetime |y:datetime |z:datetime |u:datetime
1953-09-02T00:00:00Z |1953-09-02T03:00:00Z |1953-09-02T03:00:00Z |1953-09-01T21:00:00Z |1953-09-01T21:00:00Z
;


implicitCastingArithmeticOperationAddTemporalAmountInString
required_capability: implicit_casting_string_literal_to_temporal_amount

FROM employees
| EVAL a = "1 day" + "2024-01-01",
b = "1 year" + "2024-04-01" + "1 month",
c = "2024-01-01" + "3600 seconds",
d = "2024-04-01" + ("1 year" + "1 day")
| KEEP a, b, c, d
| LIMIT 1
;

a:datetime | b:datetime | c:datetime | d:datetime
2024-01-02 | 2025-05-01 | 2024-01-01T01:00:00.000Z | 2025-04-02
;

implicitCastingArithmeticOperationSubTemporalAmountInString
required_capability: implicit_casting_string_literal_to_temporal_amount

FROM employees
| EVAL a = "2024-01-01" - "1 day",
b = "2024-04-01" - "1 month",
c = "2024-01-01" - "3600 seconds",
d = "2024-04-01" - ("1 year" + "1 day")
| KEEP a, b, c, d
| LIMIT 1
;

a:datetime | b:datetime | c:datetime | d:datetime
2023-12-31 | 2024-03-01 | 2023-12-31T23:00:00.000Z | 2023-03-31
;

implicitCastingArithmeticOperationAddSubTemporalAmountInString
required_capability: implicit_casting_string_literal_to_temporal_amount

FROM employees
| EVAL a = "1 month" + "2024-01-01" - "1 day",
b = - "1 year" + "2024-04-01" + "1 month",
c = "1 hour" + "2024-01-01" - "3600 seconds",
d = "2024-04-01" - ("1 year" + "1 day"),
e = "2024-01-01" + ("4 years" + "3 months") + "2 weeks" + "1 day",
f = "4 years" + ("3 months" + "2 weeks") + "1 day" + "2024-01-01",
g = "2024-01-01" + (-("4 years" + "3 months" + "2 weeks" + "1 day")),
h = "2024-01-01" - "4 years" - "3 months" - "2 weeks" - "1 day",
i = - "4 years" - "3 months" - "2 weeks" - "1 day" + "2024-01-01",
j = "2024-01-01" - (- "4 years" - "3 months" - "2 weeks" - "1 day"),
k = "2024-01-01" + "1 hour" + "1 minute" + "1 second" + "1 milliseconds"
| KEEP a, b, c, d, e, f, g, h, i, j, k
| LIMIT 1
;

a:datetime |b:datetime |c:datetime |d:datetime |e:datetime |f:datetime |g:datetime |h:datetime |i:datetime |j:datetime |k:datetime
2024-01-31 |2023-05-01 |2024-01-01 |2023-03-31 |2028-04-16 |2028-04-16 |2019-09-16 |2019-09-16 |2019-09-16 |2028-04-16 |2024-01-01T01:01:01.001Z
;

implicitCastingTemporalAmountInStringWithNulls
required_capability: implicit_casting_string_literal_to_temporal_amount

FROM employees
| EVAL a = to_dt(null) - "1 day", b = "2024-01-01" + null + "1 day", c = null + "1 day" + "2024-01-01"
| KEEP a, b, c
| LIMIT 1;

a:datetime |b:datetime |c:datetime
null |null |null
;

filteringWithTemporalAmountInString
required_capability: implicit_casting_string_literal_to_temporal_amount

Expand Down
112 changes: 40 additions & 72 deletions x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/analysis/Analyzer.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,8 @@
import org.elasticsearch.xpack.esql.stats.FeatureMetric;
import org.elasticsearch.xpack.esql.type.EsqlDataTypeConverter;

import java.time.Duration;
import java.time.temporal.TemporalAmount;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.BitSet;
Expand Down Expand Up @@ -120,7 +122,7 @@
import static org.elasticsearch.xpack.esql.core.type.DataType.VERSION;
import static org.elasticsearch.xpack.esql.core.type.DataType.isTemporalAmount;
import static org.elasticsearch.xpack.esql.stats.FeatureMetric.LIMIT;
import static org.elasticsearch.xpack.esql.type.EsqlDataTypeConverter.parseTemporalAmount;
import static org.elasticsearch.xpack.esql.type.EsqlDataTypeConverter.maybeParseTemporalAmount;

/**
* This class is part of the planner. Resolves references (such as variable and index names) and performs implicit casting.
Expand All @@ -145,8 +147,8 @@ public class Analyzer extends ParameterizedRuleExecutor<LogicalPlan, AnalyzerCon
var resolution = new Batch<>(
"Resolution",
/*
* Move ImplicitCasting before ResolveRefs. Because a ref is created for a Bucket in Aggregate's aggregates,
* resolving this ref before implicit casting may cause this ref to have customMessage=true, it prevents further
* ImplicitCasting must be before ResolveRefs. Because a reference is created for a Bucket in Aggregate's aggregates,
* resolving this reference before implicit casting may cause this reference to have customMessage=true, it prevents further
* attempts to resolve this reference.
*/
new ImplicitCasting(),
Expand Down Expand Up @@ -1055,33 +1057,33 @@ private static Expression processBinaryOperator(BinaryOperator<?, ?, ?, ?> o) {
}
List<Expression> newChildren = new ArrayList<>(2);
boolean childrenChanged = false;
String errorMessage = "Cannot convert string [{}] to any of [" + DATETIME + ", " + DATE_PERIOD + ", " + TIME_DURATION + "]";

if (o instanceof DateTimeArithmeticOperation) {
childrenChanged = processDateTimeArithmeticOperation(left, right, newChildren, errorMessage);
} else {
DataType targetDataType = DataType.NULL;
Expression from = Literal.NULL;

if (left.dataType() == KEYWORD && left.foldable() && (left instanceof EsqlScalarFunction == false)) {
if (supportsStringImplicitCasting(right.dataType())) {
targetDataType = right.dataType();
from = left;
}
}
if (right.dataType() == KEYWORD && right.foldable() && (right instanceof EsqlScalarFunction == false)) {
if (supportsStringImplicitCasting(left.dataType())) {
targetDataType = left.dataType();
from = right;
}
DataType targetDataType = DataType.NULL;
Expression from = Literal.NULL;

if (left.dataType() == KEYWORD && left.foldable() && (left instanceof EsqlScalarFunction == false)) {
if (supportsStringImplicitCasting(right.dataType())) {
targetDataType = right.dataType();
from = left;
} else if (supportsImplicitTemporalCasting(right, o)) {
targetDataType = DATETIME;
from = left;
}
if (from != Literal.NULL) {
Expression e = castStringLiteral(from, targetDataType);
newChildren.add(from == left ? e : left);
newChildren.add(from == right ? e : right);
childrenChanged = true;
}
if (right.dataType() == KEYWORD && right.foldable() && (right instanceof EsqlScalarFunction == false)) {
if (supportsStringImplicitCasting(left.dataType())) {
targetDataType = left.dataType();
from = right;
} else if (supportsImplicitTemporalCasting(left, o)) {
targetDataType = DATETIME;
from = right;
}
}
if (from != Literal.NULL) {
Expression e = castStringLiteral(from, targetDataType);
newChildren.add(from == left ? e : left);
newChildren.add(from == right ? e : right);
childrenChanged = true;
}
return childrenChanged ? o.replaceChildren(newChildren) : o;
}

Expand Down Expand Up @@ -1110,48 +1112,6 @@ private static Expression processIn(In in) {
return childrenChanged ? in.replaceChildren(newChildren) : in;
}

private static boolean processDateTimeArithmeticOperation(
Expression left,
Expression right,
List<Expression> newChildren,
String errorMessage
) {
boolean childrenChanged = false;
Expression newLeft = left;
Expression newRight = right;
if (isStringLiteral(left)) {
newLeft = castToTemporalOrDateTime(left, errorMessage);
}
if (isStringLiteral(right)) {
newRight = castToTemporalOrDateTime(right, errorMessage);
}
if (newLeft != left || newRight != right) {
newChildren.add(newLeft);
newChildren.add(newRight);
childrenChanged = true;
}
return childrenChanged;
}

private static boolean isStringLiteral(Expression e) {
return e instanceof Literal l && l.dataType() == KEYWORD;
}

private static Expression castToTemporalOrDateTime(Expression e, String errorMessage) {
Expression result = castStringLiteralToTemporalAmount(e); // try casting it to temporal first
if (result instanceof Literal nr && isTemporalAmount(nr.dataType())) {
return result;
} else {
result = castStringLiteral(e, DATETIME); // rhs is not a temporal, try casting it to datetime
if (result instanceof Literal nr && nr.dataType() == DATETIME) {
return result;
} else if (result instanceof UnresolvedAttribute ua) {
return ua.withUnresolvedMessage(format(errorMessage, e.fold()));
}
}
return e;
}

private static boolean canCastMixedNumericTypes(org.elasticsearch.xpack.esql.core.expression.function.Function f) {
return f instanceof Coalesce;
}
Expand Down Expand Up @@ -1191,8 +1151,12 @@ private static Expression castMixedNumericTypes(EsqlScalarFunction f, DataType t
return childrenChanged ? f.replaceChildren(newChildren) : f;
}

private static boolean supportsImplicitTemporalCasting(Expression e, BinaryOperator<?, ?, ?, ?> o) {
return isTemporalAmount(e.dataType()) && (o instanceof DateTimeArithmeticOperation);
}

private static boolean supportsStringImplicitCasting(DataType type) {
return type == DATETIME || type == IP || type == VERSION || type == BOOLEAN || isTemporalAmount(type);
return type == DATETIME || type == IP || type == VERSION || type == BOOLEAN;
}

private static UnresolvedAttribute unresolvedAttribute(Expression value, String type, Exception e) {
Expand All @@ -1207,8 +1171,12 @@ private static UnresolvedAttribute unresolvedAttribute(Expression value, String

private static Expression castStringLiteralToTemporalAmount(Expression from) {
try {
Literal result = parseTemporalAmount(from);
return result == null ? from : result;
TemporalAmount result = maybeParseTemporalAmount(from.fold().toString().strip());
if (result == null) {
return from;
}
DataType target = result instanceof Duration ? TIME_DURATION : DATE_PERIOD;
return new Literal(from.source(), result, target);
} catch (Exception e) {
return unresolvedAttribute(from, DATE_PERIOD + " or " + TIME_DURATION, e);
}
Expand Down
Loading

0 comments on commit 8b31b45

Please sign in to comment.