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

Improved Terms Aggregation documentation #38892

Merged
merged 3 commits into from
Mar 5, 2019
Merged

Improved Terms Aggregation documentation #38892

merged 3 commits into from
Mar 5, 2019

Conversation

srensamblador
Copy link
Contributor

Added a footnote explaining the need to enable fielddata for text fields, or the alternative of using keywords, for example.

Closes issue #38741

@ywelsch ywelsch added >docs General docs changes :Analytics/Aggregations Aggregations labels Feb 14, 2019
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-analytics-geo

@polyfractal
Copy link
Contributor

Hmm, I looked through the CLA database and didn't see your name/username. What email did you use? (you can shoot me an email zach@elastic.co if you don't want to paste it here :) )

}
}
}
--------------------------------------------------
// CONSOLE
// TEST[s/_search/_search\?filter_path=aggregations/]
<1> `terms` aggregation needs a field of type `keyword` or any other data type suitable for bucket aggregations. In order to use it with `text` you will need to enable
Copy link
Contributor

Choose a reason for hiding this comment

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

Could we say "should be a field of type 'keyword'" instead of "needs" ? Just because it doesn't have to be a keyword, it's just strongly suggested/preferred :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure thing, fits better indeed. I'll update it as soon as I can.

Regarding the CLA, I used this email: kaisserds@gmail.com

I did get a confirmation with a copy of the signed document when I signed it, by the way.

@srensamblador
Copy link
Contributor Author

Updated the footnote with the suggested wording

@polyfractal
Copy link
Contributor

Thanks @srensamblador. Our infra team has found an issue with the CLA signer/checker system and are investigating why yours (and other) CLA signatures aren't showing up. I'll update once we get that squared away, thanks for your patience!

@polyfractal
Copy link
Contributor

@srensamblador Ok, I think infra has fixed the CLA system. Unfortunately they said signatures that were signed during the "downtime" are unable to be imported (something to do with the interaction between echosign and our system).

Would you mind signing again? Sorry for the hassle!

@srensamblador
Copy link
Contributor Author

@polyfractal Perfect! Got it ready :)

@polyfractal
Copy link
Contributor

Woo thanks! Kicking off a quick docs CI run then we'll get this merged :)

@elasticmachine test this please

@polyfractal polyfractal merged commit ff6ffe8 into elastic:master Mar 5, 2019
@polyfractal
Copy link
Contributor

Merged, thanks again @srensamblador! Will get this backported to the relevant branches in a minute :)

polyfractal pushed a commit that referenced this pull request Mar 5, 2019
Added a note after the first query example talking about fielddata.
polyfractal pushed a commit that referenced this pull request Mar 5, 2019
Added a note after the first query example talking about fielddata.
polyfractal pushed a commit that referenced this pull request Mar 5, 2019
Added a note after the first query example talking about fielddata.
jasontedor added a commit to jasontedor/elasticsearch that referenced this pull request Mar 6, 2019
* 6.7: (39 commits)
  Remove beta label from CCR (elastic#39722)
  Rename retention lease setting (elastic#39719)
  Add Docker build type (elastic#39378)
  Use any index specified by .watches for Watcher (elastic#39541) (elastic#39706)
  Add documentation on remote recovery (elastic#39483)
  fix typo in synonym graph filter docs
  Removed incorrect ML YAML tests (elastic#39400)
  Improved Terms Aggregation documentation (elastic#38892)
  Fix Fuzziness#asDistance(String) (elastic#39643)
  Revert "unmute EvilLoggerTests#testDeprecatedSettings (elastic#38743)"
  Mute TokenAuthIntegTests.testExpiredTokensDeletedAfterExpiration (elastic#39690)
  Fix security index auto-create and state recovery race (elastic#39582)
  [DOCS] Sorts security APIs
  Check for .watches that wasn't upgraded properly (elastic#39609)
  Assert recovery done in testDoNotWaitForPendingSeqNo (elastic#39595)
  [DOCS] Updates API in Watcher transform context (elastic#39540)
  Fixing the custom object serialization bug in diffable utils. (elastic#39544)
  mute test
  SQL: Don't allow inexact fields for MIN/MAX (elastic#39563)
  Update release notes for 6.7.0
  ...
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.

5 participants