Skip to content

[ES|QL] TS Disallow renaming into timestamp prior to implicit use #137713

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.

Sign up for GitHub

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

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from
+7 −3
Draft

[ES|QL] TS Disallow renaming into timestamp prior to implicit use #137713

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
3 changes: 0 additions & 3 deletions muted-tests.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -464,9 +464,6 @@ tests:
- class: org.elasticsearch.xpack.inference.integration.CCMStorageServiceIT
method: testStoreAndGetCCMModel
issue: https://github.com/elastic/elasticsearch/issues/137630
- class: org.elasticsearch.xpack.esql.qa.single_node.GenerativeMetricsIT
method: test
issue: https://github.com/elastic/elasticsearch/issues/137655
- class: org.elasticsearch.xpack.inference.action.filter.ShardBulkInferenceActionFilterBasicLicenseIT
method: testLicenseInvalidForInference {p0=true}
issue: https://github.com/elastic/elasticsearch/issues/137690
Expand Down
1 change: 1 addition & 0 deletions ...ver/src/main/java/org/elasticsearch/xpack/esql/qa/rest/generative/GenerativeRestTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,7 @@ public abstract class GenerativeRestTest extends ESRestTestCase implements Query
"implicit time-series aggregation function .* doesn't support type .*",
"INLINE STATS .* can only be used after STATS when used with TS command",
"cannot group by a metric field .* in a time-series aggregation",
"@timestamp field has been modified",
"Output has changed from \\[.*\\] to \\[.*\\]" // https://github.com/elastic/elasticsearch/issues/134794
);

Expand Down
6 changes: 6 additions & 0 deletions ...va/org/elasticsearch/xpack/esql/optimizer/rules/logical/TranslateTimeSeriesAggregate.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
import org.elasticsearch.xpack.esql.core.type.DataType;
import org.elasticsearch.xpack.esql.core.util.CollectionUtils;
import org.elasticsearch.xpack.esql.core.util.Holder;
import org.elasticsearch.xpack.esql.expression.function.TimestampAware;
import org.elasticsearch.xpack.esql.expression.function.aggregate.AggregateFunction;
import org.elasticsearch.xpack.esql.expression.function.aggregate.DimensionValues;
import org.elasticsearch.xpack.esql.expression.function.aggregate.LastOverTime;
Expand Down Expand Up @@ -240,6 +241,11 @@ protected LogicalPlan rule(TimeSeriesAggregate aggregate, LogicalOptimizerContex
}
}
}
if (aggregate.child().output().contains(timestamp.get()) == false
&& timeSeriesAggs.keySet().stream().anyMatch(ts -> ts instanceof TimestampAware)) {
// TODO: rephrase
throw new IllegalArgumentException("@timestamp field has been modified");
}
// time-series aggregates must be grouped by _tsid (and time-bucket) first and re-group by users key
List<Expression> firstPassGroupings = new ArrayList<>();
firstPassGroupings.add(tsid.get());
Expand Down