Skip to content
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.

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

TSDB: improve error message for nested fields #83837

Merged
merged 1 commit into from
Feb 14, 2022

Conversation

nik9000
Copy link
Member

@nik9000 nik9000 commented Feb 11, 2022

Nested fields have never been supported in time_series mode but their
error message was something along the lines of "nested fields don't work
when you've enabled index sorting". Which is true, but when you put
an index into time_series mode you don't explicitly enable index sorting

  • time_series mode does that for you. So the message is confusing. This
    gives you a more explicit "nested fields don't work in time_series
    mode".

I've also added tests for nested dimensions outside of time series mode.
We didn't have any such tests and it's nice to assert how they currently
work.

@elasticmachine elasticmachine added the Team:Analytics Meta label for analytical engine team (ESQL/Aggs/Geo) label Feb 11, 2022
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-analytics-geo (Team:Analytics)

Nested fields have never been supported in time_series mode but their
error message was something along the lines of "nested fields don't work
when you've enabled index sorting". Which is *true*, but when you put
an index into time_series mode you don't explicitly enable index sorting
- time_series mode does that for you. So the message is confusing. This
gives you a more explicit "nested fields don't work in time_series
mode".

I've also added tests for nested dimensions outside of time series mode.
We didn't have any such tests and it's nice to assert how they currently
work.
Copy link
Contributor

@csoulios csoulios left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@nik9000 nik9000 merged commit 2e7ca44 into elastic:master Feb 14, 2022
@nik9000
Copy link
Member Author

nik9000 commented Feb 14, 2022

Thanks @csoulios !

nik9000 added a commit that referenced this pull request Feb 14, 2022
Like ships passing in the night #83799 and #83837 missed eachother and
then crashed together in the master branch causing failures. Ooops.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
>non-issue :StorageEngine/TSDB You know, for Metrics Team:Analytics Meta label for analytical engine team (ESQL/Aggs/Geo) v8.2.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants