Return a better error message when Timestamp is renamed in TS queries by not-napoleon · Pull Request #136231 · elastic/elasticsearch

not-napoleon · 2025-10-08T21:17:56Z

After discussing this more on #136062, we have decided queries that rename the @timestamp field should fail with a clear error message.

This relates to #136772

elasticsearchmachine · 2025-10-08T21:18:33Z

Hi @not-napoleon, I've created a changelog YAML for you.

This isn't going to work because the objects get recreated arbitrarially.

…or-message' into rename-ts-error-message

Conflicts: x-pack/plugin/esql/qa/testFixtures/src/main/resources/k8s-timeseries.csv-spec x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/analysis/VerifierTests.java x-pack/plugin/src/yamlRestTest/resources/rest-api-spec/test/esql/40_tsdb.yml

…or-message' into rename-ts-error-message

x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/analysis/VerifierTests.java

…nged" This reverts commit dbcec93.

…or-message' into rename-ts-error-message

elasticsearchmachine · 2025-10-21T20:29:27Z

Pinging @elastic/es-storage-engine (Team:StorageEngine)

dnhatn

I left some nits, but this looks good. Can you also change to UnresolvedTimestamp for LastOverTime, FirstOverTime, Increase, Irate, Delta, and IDelta? Thank you, Mark!

dnhatn · 2025-10-21T20:51:12Z

x-pack/plugin/src/yamlRestTest/resources/rest-api-spec/test/esql/40_tsdb.yml

-                            rx:
-                              type: integer
-                              time_series_metric: counter
+        index: test


this: nit: can we avoid reformatting this file?

dnhatn · 2025-10-21T20:51:52Z

.../src/test/java/org/elasticsearch/xpack/esql/optimizer/AbstractLogicalPlanOptimizerTests.java

    protected static Analyzer sampleDataIndexAnalyzer;

-    protected static EnrichResolution enrichResolution;
+    public static EnrichResolution enrichResolution;


nit: we don't need this change?

dnhatn · 2025-10-21T20:54:43Z

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

+required_capability: ts_command_v0
+
+TS k8s METADATA _tsid
+| RENAME _tsid AS newTsid


Hmm, I think these queries should fail. You don't have to handle them in this PR.

Yeah, we need to open a few follow up issues. There's also the case of shadowing timestamp in an eval that should fail.

dnhatn · 2025-10-21T20:56:08Z

...sql-core/src/main/java/org/elasticsearch/xpack/esql/core/expression/UnresolvedTimestamp.java

+
+    @Override
+    public UnresolvedTimestamp withUnresolvedMessage(String unresolvedMessage) {
+        return new UnresolvedTimestamp(source(), qualifier(), name(), id(), unresolvedMessage, resolutionMetadata(), errorMessage);


If you don't want to override String unresolvedMessage(), you can override unresolvedMessage here instead. Either approach is good.

Conflicts: x-pack/plugin/esql-core/src/main/java/org/elasticsearch/xpack/esql/core/expression/UnresolvedAttribute.java x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/analysis/VerifierTests.java

Conflicts: x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/analysis/VerifierTests.java

not-napoleon · 2025-10-22T19:16:29Z

@elasticmachine run elasticsearch-ci/part-3

elasticsearchmachine · 2025-10-24T13:54:31Z

💔 Backport failed

Status	Branch	Result
❌	9.2	Commit could not be cherrypicked due to conflicts

You can use sqren/backport to manually backport by running backport --upstream elastic/elasticsearch --pr 136231

@timestamp

…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 <infra-root+elasticsearchmachine@elastic.co> Conflicts: x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/analysis/VerifierTests.java

@timestamp

…ueries (#136231) (#137112) * Return a better error message when Timestamp is renamed in TS queries (#136231) Resolves #134994 After discussing this more on #136062, we have decided queries that rename the @timestamp field should fail with a clear error message. This relates to #136772 --------- Co-authored-by: elasticsearchmachine <infra-root+elasticsearchmachine@elastic.co> 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 <infra-root+elasticsearchmachine@elastic.co>

not-napoleon · 2025-10-24T18:54:30Z

manual backport PR: #137112

@timestamp

…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 <infra-root+elasticsearchmachine@elastic.co>

not-napoleon added 3 commits October 8, 2025 15:34

first pass

50cf746

another test case

5e439ea

mirror the expected behavior

0cd6ec7

not-napoleon added >bug auto-backport Automatically create backport pull requests when merged v9.2.0 :StorageEngine/ES|QL Timeseries / metrics / PromQL / logsdb capabilities in ES|QL v9.3.0 labels Oct 8, 2025

Update docs/changelog/136231.yaml

3235396

not-napoleon and others added 12 commits October 16, 2025 10:22

Revert override plan

be0cb45

This isn't going to work because the objects get recreated arbitrarially.

Merge remote-tracking branch 'refs/remotes/not-napoleon/rename-ts-err…

882f662

…or-message' into rename-ts-error-message

tests, and unnecessary yaml reformatting (thanks Intellij)

3404b20

ugly, but this works

778358a

[CI] Auto commit changes from spotless

b4155ed

Merge remote-tracking branch 'refs/remotes/not-napoleon/rename-ts-err…

a2fe556

…or-message' into rename-ts-error-message

[CI] Auto commit changes from spotless

65c4ee1

fix creating analyzer

c84651b

Merge branch 'main' into rename-ts-error-message

6ad4378

'fix' the verifier tests, but really these shouldn't have changed

dbcec93

make sure we don't return a message if the unresolved message is null

c2d9266

not-napoleon commented Oct 21, 2025

View reviewed changes

x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/analysis/VerifierTests.java Outdated Show resolved Hide resolved

not-napoleon and others added 7 commits October 21, 2025 10:15

Revert "'fix' the verifier tests, but really these shouldn't have cha…

f74238e

…nged" This reverts commit dbcec93.

revert verifier test 'fix', try to fix this correctly

43c4448

this doesn't work either

d712424

Nhat's suggestion

7c50fb0

[CI] Auto commit changes from spotless

78b67c3

fix node subclass tests

2169c6b

Merge remote-tracking branch 'refs/remotes/not-napoleon/rename-ts-err…

b53625f

…or-message' into rename-ts-error-message

I don't understand how this ever worked without this

b900656

not-napoleon marked this pull request as ready for review October 21, 2025 20:29

elasticsearchmachine added the Team:StorageEngine label Oct 21, 2025

dnhatn approved these changes Oct 21, 2025

View reviewed changes

not-napoleon added 5 commits October 22, 2025 11:27

Merge branch 'main' into rename-ts-error-message

c0cb608

Conflicts: x-pack/plugin/esql-core/src/main/java/org/elasticsearch/xpack/esql/core/expression/UnresolvedAttribute.java x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/analysis/VerifierTests.java

other cleanups

b27b725

apply custom error message to other TS functions

9e856b6

don't let intellij reformat the yml tests

e3a0afd

Merge branch 'main' into rename-ts-error-message

d8e4ed7

Conflicts: x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/analysis/VerifierTests.java

not-napoleon enabled auto-merge (squash) October 22, 2025 18:16

[CI] Auto commit changes from spotless

3b9d1e3

not-napoleon added 3 commits October 23, 2025 08:36

Merge branch 'main' into rename-ts-error-message

f541615

Merge branch 'main' into rename-ts-error-message

ffe8e52

creating test analyzers is fragile apparently

33f9b7e

not-napoleon added branch:9.2 and removed v9.2.0 labels Oct 24, 2025

elasticsearchmachine added v9.2.1 and removed branch:9.2 labels Oct 24, 2025

Merge branch 'main' into rename-ts-error-message

f5c9e24

not-napoleon merged commit ef9199d into elastic:main Oct 24, 2025
34 checks passed

elasticsearchmachine added the backport pending label Oct 24, 2025

not-napoleon removed the backport pending label Oct 24, 2025

not-napoleon mentioned this pull request Oct 28, 2025

ESQL Fix bug when renaming @timestamp in TS queries #136062

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Return a better error message when Timestamp is renamed in TS queries#136231

Return a better error message when Timestamp is renamed in TS queries#136231
not-napoleon merged 34 commits intoelastic:mainfrom
not-napoleon:rename-ts-error-message

not-napoleon commented Oct 8, 2025 •

edited

Loading

elasticsearchmachine commented Oct 8, 2025

Uh oh!

elasticsearchmachine commented Oct 21, 2025

dnhatn left a comment

dnhatn Oct 21, 2025

dnhatn Oct 21, 2025

dnhatn Oct 21, 2025

not-napoleon Oct 22, 2025

dnhatn Oct 21, 2025

not-napoleon commented Oct 22, 2025

Uh oh!

elasticsearchmachine commented Oct 24, 2025

not-napoleon commented Oct 24, 2025

Labels

3 participants

Conversation

not-napoleon commented Oct 8, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

elasticsearchmachine commented Oct 8, 2025

Uh oh!

elasticsearchmachine commented Oct 21, 2025

dnhatn left a comment

Choose a reason for hiding this comment

dnhatn Oct 21, 2025

Choose a reason for hiding this comment

dnhatn Oct 21, 2025

Choose a reason for hiding this comment

dnhatn Oct 21, 2025

Choose a reason for hiding this comment

not-napoleon Oct 22, 2025

Choose a reason for hiding this comment

dnhatn Oct 21, 2025

Choose a reason for hiding this comment

not-napoleon commented Oct 22, 2025

Uh oh!

elasticsearchmachine commented Oct 24, 2025

💔 Backport failed

not-napoleon commented Oct 24, 2025

Labels

3 participants

not-napoleon commented Oct 8, 2025 •

edited

Loading