Skip to content

Commit ef9199d

Browse files
not-napoleonelasticsearchmachine
not-napoleon
and
elasticsearchmachine
authored
Return a better error message when Timestamp is renamed in TS queries (elastic#136231)
Resolves elastic#134994 After discussing this more on elastic#136062, we have decided queries that rename the @timestamp field should fail with a clear error message. This relates to elastic#136772 --------- Co-authored-by: elasticsearchmachine <[email protected]>
1 parent f40259a commit ef9199d

File tree

18 files changed

+355
-23
lines changed

18 files changed

+355
-23
lines changed

‎docs/changelog/136231.yaml‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
pr: 136231
2+
summary: Return a better error message when Timestamp is renamed in TS queries
3+
area: ES|QL
4+
type: bug
5+
issues:
6+
- 134994

‎x-pack/plugin/esql-core/src/main/java/org/elasticsearch/xpack/esql/core/capabilities/Unresolvable.java‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,5 +15,8 @@ default boolean resolved() {
1515
return false;
1616
}
1717

18+
/**
19+
* NOTE: Any non-null return value from this method indicates that the item in question could not be resolved.
20+
*/
1821
String unresolvedMessage();
1922
}

‎x-pack/plugin/esql-core/src/main/java/org/elasticsearch/xpack/esql/core/expression/UnresolvedAttribute.java‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -31,9 +31,9 @@
3131
*/
3232
public class UnresolvedAttribute extends Attribute implements Unresolvable {
3333
private final boolean customMessage;
34-
private final String unresolvedMsg;
34+
protected final String unresolvedMsg;
3535
// TODO: unused and always null, remove this.
36-
private final Object resolutionMetadata;
36+
protected final Object resolutionMetadata;
3737

3838
// TODO: Check usage of constructors without qualifiers, that's likely where qualifiers need to be plugged into resolution logic.
3939
public UnresolvedAttribute(Source source, String name) {
@@ -86,7 +86,7 @@ public String getWriteableName() {
8686
}
8787

8888
@Override
89-
protected NodeInfo<UnresolvedAttribute> info() {
89+
protected NodeInfo<? extends UnresolvedAttribute> info() {
9090
return NodeInfo.create(this, UnresolvedAttribute::new, qualifier(), name(), id(), unresolvedMsg, resolutionMetadata);
9191
}
9292

‎x-pack/plugin/esql-core/src/main/java/org/elasticsearch/xpack/esql/core/expression/UnresolvedTimestamp.java‎

Lines changed: 71 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,71 @@
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+
* 2.0; you may not use this file except in compliance with the Elastic License
5+
* 2.0.
6+
*/
7+
8+
package org.elasticsearch.xpack.esql.core.expression;
9+
10+
import org.elasticsearch.xpack.esql.core.tree.NodeInfo;
11+
import org.elasticsearch.xpack.esql.core.tree.Source;
12+
13+
import java.util.Objects;
14+
15+
public class UnresolvedTimestamp extends UnresolvedAttribute {
16+
private final String errorMessage;
17+
18+
public UnresolvedTimestamp(Source source, String errorMessage) {
19+
this(source, null, MetadataAttribute.TIMESTAMP_FIELD, null, null, null, errorMessage);
20+
}
21+
22+
public UnresolvedTimestamp(
23+
Source source,
24+
String qualifier,
25+
String name,
26+
NameId id,
27+
String unresolvedMessage,
28+
Object resolutionMetadata,
29+
String errorMessage
30+
) {
31+
super(source, qualifier, name, id, unresolvedMessage, resolutionMetadata);
32+
this.errorMessage = errorMessage;
33+
}
34+
35+
@Override
36+
protected NodeInfo<UnresolvedTimestamp> info() {
37+
return NodeInfo.create(
38+
this,
39+
UnresolvedTimestamp::new,
40+
qualifier(),
41+
name(),
42+
id(),
43+
super.unresolvedMessage(),
44+
resolutionMetadata(),
45+
errorMessage
46+
);
47+
}
48+
49+
@Override
50+
public UnresolvedTimestamp withUnresolvedMessage(String unresolvedMessage) {
51+
return new UnresolvedTimestamp(source(), qualifier(), name(), id(), unresolvedMessage, resolutionMetadata(), errorMessage);
52+
}
53+
54+
@Override
55+
public String unresolvedMessage() {
56+
if (super.unresolvedMessage() != null) {
57+
return errorMessage;
58+
}
59+
return null;
60+
}
61+
62+
@Override
63+
protected int innerHashCode(boolean ignoreIds) {
64+
return Objects.hash(super.innerHashCode(ignoreIds), errorMessage);
65+
}
66+
67+
@Override
68+
protected boolean innerEquals(Object o, boolean ignoreIds) {
69+
return super.innerEquals(o, ignoreIds) && Objects.equals(errorMessage, ((UnresolvedTimestamp) o).errorMessage);
70+
}
71+
}

‎x-pack/plugin/esql/qa/testFixtures/src/main/resources/k8s-timeseries.csv-spec‎

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -593,3 +593,52 @@ max_cost:integer | pod:keyword | time_bucket:datetime | max_cluster_cost:int
593593
1209 | three | 2024-05-10T00:00:00.000Z | 1209 | staging
594594
973 | two | 2024-05-10T00:00:00.000Z | 1209 | staging
595595
;
596+
597+
Max of Rate with Bucket
598+
required_capability: ts_command_v0
599+
600+
TS k8s
601+
| STATS maxRate = max(rate(network.total_cost)) BY tbucket = bucket(@timestamp, 1hour)
602+
;
603+
604+
maxRate:double | tbucket:datetime
605+
0.058979885057471274 | 2024-05-10T00:00:00.000Z
606+
;
607+
608+
Max of Rate with Bucket, rename _tsid
609+
required_capability: ts_command_v0
610+
611+
TS k8s METADATA _tsid
612+
| RENAME _tsid AS newTsid
613+
| STATS maxRate = max(rate(network.total_cost)) BY tbucket = bucket(@timestamp, 1hour)
614+
;
615+
616+
maxRate:double | tbucket:datetime
617+
0.058979885057471274 | 2024-05-10T00:00:00.000Z
618+
;
619+
620+
Max of Rate with Bucket, drop _tsid
621+
required_capability: ts_command_v0
622+
623+
TS k8s METADATA _tsid
624+
| DROP _tsid
625+
| STATS maxRate = max(rate(network.total_cost)) BY tbucket = bucket(@timestamp, 1hour)
626+
;
627+
628+
maxRate:double | tbucket:datetime
629+
0.058979885057471274 | 2024-05-10T00:00:00.000Z
630+
;
631+
632+
633+
Rename timestamp when aggs don't require timestamp
634+
required_capability: ts_command_v0
635+
636+
TS k8s
637+
| RENAME @timestamp AS newTs
638+
| STATS mx = max(max_over_time(network.eth0.tx)) BY tbucket = bucket(newTs, 1hour)
639+
;
640+
641+
mx:integer | tbucket:datetime
642+
1716 | 2024-05-10T00:00:00.000Z
643+
;
644+

‎x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/action/EsqlCapabilities.java‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1461,6 +1461,10 @@ public enum Cap {
14611461
*/
14621462
TS_COMMAND_V0(),
14631463

1464+
/**
1465+
* Custom error for renamed timestamp
1466+
*/
1467+
TS_RENAME_TIMESTAMP_ERROR_MESSAGE,
14641468
/**
14651469
* Add support for counter doubles, ints, and longs in first_ and last_over_time
14661470
*/

‎x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/analysis/Analyzer.java‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1137,6 +1137,7 @@ private Attribute maybeResolveAttribute(UnresolvedAttribute ua, List<Attribute>
11371137
}
11381138

11391139
private static Attribute maybeResolveAttribute(UnresolvedAttribute ua, List<Attribute> childrenOutput, Logger logger) {
1140+
// if we already tried and failed to resolve this attribute, don't try again
11401141
if (ua.customMessage()) {
11411142
return ua;
11421143
}
@@ -1149,7 +1150,7 @@ private Attribute resolveAttribute(UnresolvedAttribute ua, List<Attribute> child
11491150

11501151
private static Attribute resolveAttribute(UnresolvedAttribute ua, List<Attribute> childrenOutput, Logger logger) {
11511152
Attribute resolved = ua;
1152-
var named = resolveAgainstList(ua, childrenOutput);
1153+
List<Attribute> named = resolveAgainstList(ua, childrenOutput);
11531154
// if resolved, return it; otherwise keep it in place to be resolved later
11541155
if (named.size() == 1) {
11551156
resolved = named.get(0);

‎x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/expression/function/aggregate/Delta.java‎

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@
1616
import org.elasticsearch.xpack.esql.EsqlIllegalArgumentException;
1717
import org.elasticsearch.xpack.esql.core.expression.Expression;
1818
import org.elasticsearch.xpack.esql.core.expression.Literal;
19-
import org.elasticsearch.xpack.esql.core.expression.UnresolvedAttribute;
19+
import org.elasticsearch.xpack.esql.core.expression.UnresolvedTimestamp;
2020
import org.elasticsearch.xpack.esql.core.tree.NodeInfo;
2121
import org.elasticsearch.xpack.esql.core.tree.Source;
2222
import org.elasticsearch.xpack.esql.core.type.DataType;
@@ -51,7 +51,11 @@ public class Delta extends TimeSeriesAggregateFunction implements OptionalArgume
5151
examples = { @Example(file = "k8s-timeseries-delta", tag = "delta") }
5252
)
5353
public Delta(Source source, @Param(name = "field", type = { "long", "integer", "double" }) Expression field) {
54-
this(source, field, new UnresolvedAttribute(source, "@timestamp"));
54+
this(
55+
source,
56+
field,
57+
new UnresolvedTimestamp(source, "Delta aggregation requires @timestamp field, but @timestamp was renamed or dropped")
58+
);
5559
}
5660

5761
public Delta(Source source, @Param(name = "field", type = { "long", "integer", "double" }) Expression field, Expression timestamp) {

‎x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/expression/function/aggregate/FirstOverTime.java‎

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@
1717
import org.elasticsearch.xpack.esql.EsqlIllegalArgumentException;
1818
import org.elasticsearch.xpack.esql.core.expression.Expression;
1919
import org.elasticsearch.xpack.esql.core.expression.Literal;
20-
import org.elasticsearch.xpack.esql.core.expression.UnresolvedAttribute;
20+
import org.elasticsearch.xpack.esql.core.expression.UnresolvedTimestamp;
2121
import org.elasticsearch.xpack.esql.core.tree.NodeInfo;
2222
import org.elasticsearch.xpack.esql.core.tree.Source;
2323
import org.elasticsearch.xpack.esql.core.type.DataType;
@@ -60,7 +60,11 @@ public FirstOverTime(
6060
Source source,
6161
@Param(name = "field", type = { "counter_long", "counter_integer", "counter_double", "long", "integer", "double" }) Expression field
6262
) {
63-
this(source, field, new UnresolvedAttribute(source, "@timestamp"));
63+
this(
64+
source,
65+
field,
66+
new UnresolvedTimestamp(source, "First Over Time aggregation requires @timestamp field, but @timestamp was renamed or dropped")
67+
);
6468
}
6569

6670
public FirstOverTime(Source source, Expression field, Expression timestamp) {

‎x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/expression/function/aggregate/Idelta.java‎

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@
1616
import org.elasticsearch.xpack.esql.EsqlIllegalArgumentException;
1717
import org.elasticsearch.xpack.esql.core.expression.Expression;
1818
import org.elasticsearch.xpack.esql.core.expression.Literal;
19-
import org.elasticsearch.xpack.esql.core.expression.UnresolvedAttribute;
19+
import org.elasticsearch.xpack.esql.core.expression.UnresolvedTimestamp;
2020
import org.elasticsearch.xpack.esql.core.tree.NodeInfo;
2121
import org.elasticsearch.xpack.esql.core.tree.Source;
2222
import org.elasticsearch.xpack.esql.core.type.DataType;
@@ -53,7 +53,11 @@ public class Idelta extends TimeSeriesAggregateFunction implements OptionalArgum
5353
examples = { @Example(file = "k8s-timeseries-idelta", tag = "idelta") }
5454
)
5555
public Idelta(Source source, @Param(name = "field", type = { "long", "integer", "double" }) Expression field) {
56-
this(source, field, new UnresolvedAttribute(source, "@timestamp"));
56+
this(
57+
source,
58+
field,
59+
new UnresolvedTimestamp(source, "IDelta aggregation requires @timestamp field, but @timestamp was renamed or dropped")
60+
);
5761
}
5862

5963
public Idelta(Source source, @Param(name = "field", type = { "long", "integer", "double" }) Expression field, Expression timestamp) {

0 commit comments

Comments
 (0)