Skip to content

Deprecate dots in aggregation names #31468

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

Closed

Conversation

polyfractal
Copy link
Contributor

Dots in aggregation names can lead to tricky parsing situations, like being unable to sort by agg names with dots. Rather than introducing a lot of complex parsing code, we should just deprecate dots in agg names. This puts them inline with the restrictions on field names too.

This PR adds a deprecation logger and a note in the docs, letting us remove them in 8.0.

It isn't strictly a 7.0 deprecation (I would like to backport it to 6.4), but I thought it made sense to include in the 7.0 migration guide in case people upgrade directly from 6.3 to 7.0

Related to #17600 and #19040

Dots in aggregation names can lead to tricky parsing situations, like
being unable to sort by agg names with dots.

This adds a deprecation logger and a note in the docs, letting us
remove them in 8.0.

Related to elastic#17600 and elastic#19040
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-search-aggs

@rjernst
Copy link
Member

rjernst commented Jun 20, 2018

Given that 7.0 is still a long way out, why isn't the deprecation intended for 6.4 long enough to remove in 7.0? Waiting until 8.0 is a very long time.

@polyfractal
Copy link
Contributor Author

The thinking was that it's a relatively low-priority deprecation (cleans things up, but isn't necessarily blocking anything), but might affect a lot of users. So someone upgrading directly from 6.3 to 7.0 wouldn't see the deprecations and would be hit with the break.

/cc @jpountz might have some thoughts.

@polyfractal polyfractal added v6.5.0 and removed v6.4.0 labels Aug 21, 2018
@rjernst rjernst removed the review label Oct 10, 2018
@colings86 colings86 added v6.6.0 and removed v6.5.0 labels Oct 25, 2018
@colings86
Copy link
Contributor

@polyfractal is this still something we should do?

@polyfractal
Copy link
Contributor Author

Stale, closing for now. We should probably still do this but I don't have bandwidth at the moment to revisit this.

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

Successfully merging this pull request may close these issues.

4 participants