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

Add histogram datatype #4367

Merged
merged 2 commits into from
Feb 10, 2020
Merged

Add histogram datatype #4367

merged 2 commits into from
Feb 10, 2020

Conversation

russcam
Copy link
Contributor

@russcam russcam commented Feb 5, 2020

This commit adds the histogram datatype to the client. The histogram datatype
is available in Elasticsearch 7.6.0+ with at least a basic license level

Closes #4358

This commit adds the histogram datatype to the client. The histogram datatype
is available in Elasticsearch 7.6.0+ with at least a basic license level

Closes #4358
@russcam
Copy link
Contributor Author

russcam commented Feb 5, 2020

I did consider adding a type to be able to index histogram values and counts. Thoughts?

@russcam russcam mentioned this pull request Feb 5, 2020
Copy link
Contributor

@codebrain codebrain left a comment

Choose a reason for hiding this comment

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

LGTM!

@codebrain
Copy link
Contributor

The docs (https://www.elastic.co/guide/en/elasticsearch/reference/7.x/histogram.html) indicate the format for indexing is quite simple:

PUT my_index/_doc/1
{
  "my_text" : "histogram_1",
  "my_histogram" : {
      "values" : [0.1, 0.2, 0.3, 0.4, 0.5], 
      "counts" : [3, 7, 23, 12, 6] 
   }
}

I think a type to index this is probably worth it and quite simple to implement?

@Mpdreamz
Copy link
Member

Mpdreamz commented Feb 7, 2020

  • Test failures are fixed in 7.x upstream

A type used for indexing would be good, however we would also need to make sure it works well with Json.NET as the source serializer.

@russcam
Copy link
Contributor Author

russcam commented Feb 10, 2020

I think a type to index this is probably worth it and quite simple to implement?

It being a simple type was my reason to not implement it 😄 Could add one in a follow up PR?

@russcam russcam merged commit 6b3f816 into 7.x Feb 10, 2020
russcam added a commit that referenced this pull request Feb 10, 2020
This commit adds the histogram datatype to the client. The histogram datatype
is available in Elasticsearch 7.6.0+ with at least a basic license level

Closes #4358

(cherry picked from commit 6b3f816)
@Mpdreamz Mpdreamz deleted the feature/7x/histogram-type branch February 10, 2020 10:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants