Skip to content

Conversation

@jrodewig
Copy link
Contributor

@jrodewig jrodewig commented Sep 22, 2021

Adds an 8.0 breaking change for the removal of the _term and _time
agg order keys.

Relates to #39450

Preview

https://elasticsearch_78209.docs-preview.app.elstc.co/guide/en/elasticsearch/reference/master/migrating-8.0.html#breaking_80_aggregations_changes

Adds an 8.0 breaking change for the removal of the `_term` and `_time`
agg `order` keys.

Relates to #39450
@jrodewig jrodewig marked this pull request as ready for review September 22, 2021 19:33
@jrodewig jrodewig added the :Analytics/Aggregations Aggregations label Sep 22, 2021
@elasticmachine elasticmachine added the Team:Analytics Meta label for analytical engine team (ESQL/Aggs/Geo) label Sep 22, 2021
@jrodewig jrodewig requested a review from nik9000 September 22, 2021 19:34
@elasticmachine
Copy link
Collaborator

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

@not-napoleon
Copy link
Member

Should we (do we?) support this in 7.x REST compatibility mode? IIRC that didn't exist when we dropped these sort keys

@nik9000
Copy link
Member

nik9000 commented Sep 22, 2021

Should we (do we?) support this in 7.x REST compatibility mode? IIRC that didn't exist when we dropped these sort keys

I think we probably should, yeah. I think the docs change is still right even so.

@jrodewig
Copy link
Contributor Author

Should we (do we?) support this in 7.x REST compatibility mode? IIRC that didn't exist when we dropped these sort keys

Thanks for calling this out @not-napoleon. It looks like we do support them with version compatibility headers. However, I don't think we need to call that out in this breaking change. IMO that's sorta the same as using a previous version of the API.

if (parser.getRestApiVersion() == RestApiVersion.V_7 && ("_term".equals(orderKey) || "_time".equals(orderKey))) {

Copy link
Member

@not-napoleon not-napoleon left a comment

Choose a reason for hiding this comment

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

Yes, sorry, docs change is fine. I think we just need to also do REST compatibility.

@jrodewig jrodewig merged commit 15baf40 into elastic:master Sep 22, 2021
@jrodewig jrodewig deleted the docs__remove-_time_term-agg-order branch September 22, 2021 19:54
@nik9000
Copy link
Member

nik9000 commented Sep 22, 2021

It looks like we do support them with version compatibility headers.

Nice! I'd assumed we dumped them. I'm so happy we still have that. Now I can GC _term and _time from my brain again.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

:Analytics/Aggregations Aggregations >non-issue Team:Analytics Meta label for analytical engine team (ESQL/Aggs/Geo) v8.0.0-beta1

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants