Skip to content

Commit 8c89c26

Browse files
not-napoleonelasticsearchmachine
not-napoleon
and
elasticsearchmachine
authored
[9.2] Return a better error message when Timestamp is renamed in TS queries (elastic#136231) (elastic#137112)
* 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]> Conflicts: x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/analysis/VerifierTests.java * creating test analyzers is fragile apparently * [CI] Auto commit changes from spotless --------- Co-authored-by: elasticsearchmachine <[email protected]>
1 parent f14175a commit 8c89c26

File tree

18 files changed

+353
-23
lines changed

18 files changed

+353
-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
@@ -581,3 +581,52 @@ max_cost:integer | pod:keyword | time_bucket:datetime | max_cluster_cost:int
581581
1209 | three | 2024-05-10T00:00:00.000Z | 1209 | staging
582582
973 | two | 2024-05-10T00:00:00.000Z | 1209 | staging
583583
;
584+
585+
Max of Rate with Bucket
586+
required_capability: ts_command_v0
587+
588+
TS k8s
589+
| STATS maxRate = max(rate(network.total_cost)) BY tbucket = bucket(@timestamp, 1hour)
590+
;
591+
592+
maxRate:double | tbucket:datetime
593+
0.058979885057471274 | 2024-05-10T00:00:00.000Z
594+
;
595+
596+
Max of Rate with Bucket, rename _tsid
597+
required_capability: ts_command_v0
598+
599+
TS k8s METADATA _tsid
600+
| RENAME _tsid AS newTsid
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, drop _tsid
609+
required_capability: ts_command_v0
610+
611+
TS k8s METADATA _tsid
612+
| DROP _tsid
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+
621+
Rename timestamp when aggs don't require timestamp
622+
required_capability: ts_command_v0
623+
624+
TS k8s
625+
| RENAME @timestamp AS newTs
626+
| STATS mx = max(max_over_time(network.eth0.tx)) BY tbucket = bucket(newTs, 1hour)
627+
;
628+
629+
mx:integer | tbucket:datetime
630+
1716 | 2024-05-10T00:00:00.000Z
631+
;
632+

‎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
@@ -1572,6 +1572,10 @@ public enum Cap {
15721572
*/
15731573
TS_COMMAND_V0(),
15741574

1575+
/**
1576+
* Custom error for renamed timestamp
1577+
*/
1578+
TS_RENAME_TIMESTAMP_ERROR_MESSAGE,
15751579
/**
15761580
* Add support for counter doubles, ints, and longs in first_ and last_over_time
15771581
*/

‎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
@@ -1071,6 +1071,7 @@ private Attribute maybeResolveAttribute(UnresolvedAttribute ua, List<Attribute>
10711071
}
10721072

10731073
private static Attribute maybeResolveAttribute(UnresolvedAttribute ua, List<Attribute> childrenOutput, Logger logger) {
1074+
// if we already tried and failed to resolve this attribute, don't try again
10741075
if (ua.customMessage()) {
10751076
return ua;
10761077
}
@@ -1083,7 +1084,7 @@ private Attribute resolveAttribute(UnresolvedAttribute ua, List<Attribute> child
10831084

10841085
private static Attribute resolveAttribute(UnresolvedAttribute ua, List<Attribute> childrenOutput, Logger logger) {
10851086
Attribute resolved = ua;
1086-
var named = resolveAgainstList(ua, childrenOutput);
1087+
List<Attribute> named = resolveAgainstList(ua, childrenOutput);
10871088
// if resolved, return it; otherwise keep it in place to be resolved later
10881089
if (named.size() == 1) {
10891090
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)