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

Nested aggregations support for Kibana 2.X off of master #5411

Closed
wants to merge 2 commits into from
Closed

Nested aggregations support for Kibana 2.X off of master #5411

wants to merge 2 commits into from

Conversation

ppadovani
Copy link

Based on the work from this pull request:

#4806

Here is the rebased code against master.

@elasticsearch-release
Copy link

Jenkins standing by to test this. If you aren't a maintainer, you can ignore this comment. Someone with commit access, please review this and clear it for Jenkins to run; then say 'jenkins, test it'.

@exinferis
Copy link

Any comment on when this will be merged? Nested queries are a serious showstopper for quite a lot of people who want to use kibana with elastic.

@pmoust
Copy link
Member

pmoust commented Dec 8, 2015

Yeah, this s a showstopper for us as well. We 've reached to a point that we are considering forking Kibana just to have this PR merged in our branch.

@tamalnath
Copy link

In which version this feature will come ?

@tomfotherby
Copy link

+1

@spyros3
Copy link

spyros3 commented Dec 8, 2015

+1 This is really important for us. The previous issue #4806 that this one is based was tagged for 4.4.0, is this still true for this issue?

@tbragin tbragin added review and removed Team:Docs labels Dec 8, 2015
ppadovani added 2 commits December 8, 2015 09:17
@ppadovani
Copy link
Author

FYI - I just rebased against master again today and pushed.

@rashidkpc
Copy link
Contributor

Taking a look at this now. Its fairly awkward to use.

It ignores the effect that adding a nested level has on the aggregation request tree. If you have a terms aggregation that is nested, and a filter aggregation under it, it will work fine. If you move the terms agg down, it will break, because the nested level has been moved down. This is noted in the help text, but still feels awkward.

Also feels like we could do this somewhat automatically by telling the user when they've selected a field that exists under a nested one and presenting the checkbox at that point.

@ppadovani
Copy link
Author

When I last looked at what data was discovered by Kibana when an index pattern was created, nothing around what fields/mappings were nested was captured. If that data had been captured, most of the code needed for managing nested aggregations could be automatic and hidden from the user. This was our attempt at working around the existing limitation without making a huge change.

Conceivably as a version 1 this would be a stop gap, and once internal knowledge around nested fields in the mappings within the index pattern exists, the nested path field in the advanced section could simply disappear.

Of course I could be wrong... :-)

@rashidkpc
Copy link
Contributor

Correct, the nested mapping is not captured in the Kibana index pattern.

Other issues include click filtering not working due to the lack of a nested filter. Also the query box doesn't work on any nest fields unless you type it out in JSON, which is pretty nasty. There's a lot of things that would need to change to bring nested support into Kibana cleanly and be able to make the statement that it is supported.

I appreciate the work that went into this, it just isn't the direction we want to go in. At this point we're going to stick with not supporting nested unless Elasticsearch, at some point, would allow for entirely transparent interaction with nested fields, and I'm not even entirely sure thats possible.

@rashidkpc rashidkpc closed this Dec 8, 2015
@pmoust
Copy link
Member

pmoust commented Dec 8, 2015

At this point we're going to stick with not supporting nested unless Elasticsearch, at some point, would allow for entirely transparent interaction with nested fields, and I'm not even entirely sure thats possible.

@rashidkpc: Could you elaborate on 'transparent interaction with nested fields'?

@rashidkpc
Copy link
Contributor

Elasticsearch knows the mappings better than we do, in theory it could treat objects under nested fields as nested documents in every case, which wouldn't require any changes to Kibana

@rashidkpc rashidkpc removed their assignment Dec 8, 2015
@tarass
Copy link

tarass commented Dec 8, 2015

Transparent handling of nested fields by elasticsearch would be nice, but that would push this feature out even longer into the future.

Is there any possibility of adding this support in the form of a plug-in so that native Kibana support isn't a requirement? (I don't know enough about Kibana plug-in architecture)

@pmoust
Copy link
Member

pmoust commented Dec 8, 2015

I echo the same

@tamalnath
Copy link

@rashidkpc: I request you to go ahead with your pull request for to be merged. If elasticsearch in future treats array of objects as nested documents, you can remove the changes for nested documents.

Not supporting nested documents is a major cause of performance issues and unwanted complexity in our project.

@chenryn
Copy link
Contributor

chenryn commented Dec 9, 2015

How about add a new field in index pattern to record the nested informations? Waiting ES to change the object design maybe so long time...

@ppadovani
Copy link
Author

There IS a solution here, but it would require a significant amount of work and I just don't have the time. We implemented this in JAVA so I know this is possible.

  1. Each index mapping needs to pull and understand nested fields.
  2. Build a custom AST that provides a simplified query language instead of attempting to just use Elasticsearch's.
  3. Build a query adapter that understands the AST and can validate and convert the query to the proper JSON.
  4. Update the aggregations in Kibana to properly handle nested fields based on the internal understanding of nested fields.

This is not impossible to do, just requires significant work. The benefits for implementing the above include query validation and a simplified syntax. For example our AST allows us to create a query like this:

(owner.user = "/users/00a0/18066271-29f0-40af-83ad-e250c8fc5944") AND (druid = "/druids/0060/77dd14b1-b7f0-4801-9ef8-74daa1859d4d") AND ((owner.lastMessageReceived.inserted >= 0) OR (conversationLifecycleState = "RESERVATION_REQUEST")) AND (owner.conversationArchived = false) AND ((units.site IS NULL) OR (units.site IN {"HOMEAWAY_DE"})) AND ((inquiry.inserted >= 0) OR NOT (reservation.availabilityStatus = "DELETE")) AND NOT (owner.markedSpam = true) AND (lastMessage.inserted >= 0)

How exactly could I make this query with the existing language?

IMHO - waiting for nirvana from Elasticsearch before acting will lead to a drop in adoption and loss of users of Kibana.

@Tybielee
Copy link

This is really unfortunate to see that this is no longer on the roadmap for future Kibana releases. I have been hoping that nested visualizations would be added to Kibana soon to fully adopt the usage of Elasticsearch and Kibana for a visualization tool.

@aldanor
Copy link

aldanor commented Dec 15, 2015

A real shame this has been declined, a big gap for Kibana and a reason we can't switch over to it.

@aodj
Copy link

aodj commented Dec 22, 2015

I have to echo the sentiment above. Given that nested fields are recommended by the ElasticSearch documentation as a good approach for reducing memory overhead and managing index growth, lack of support in Kibana isn't just affecting Kibana's usage but the usage of this entire feature in ElasticSearch; why use a feature in ElasticSearch when the recommended visualisation software developed by the same company doesn't support it?

@tarass
Copy link

tarass commented Dec 22, 2015

I've been successfully running ppadovani fork and it works beautifully.

@rashidkpc I'd like to point out one of the usecases for nested objects is not classic relational data, it is basic support for arrays of objects.
Consider this log entry:
{"action": "view", "modules":[{"id": "abc", "position": 1}, {"id":"xyz", "position":2}]}

@rashidkpc
Copy link
Contributor

Don't get me wrong, I too would like nested support in Kibana, there's just a ton of other things that also need work and the amount of work required to build out nested support for Kibana in a cohesive way would be massive. For example, Elasticsearch doesn't support nested in query_string, so it would require a new search language within Kibana, no small undertaking, and only one small part of the picture.

@tamalnath
Copy link

I personally feel that elasticsearch should change the API so that nested query and nested aggregation should work as regular query and aggregation. In that case, no change in kibana is required. I don't know if there is any technical limitation on that.

@rashidkpc: Can you please prioritize nested aggregation in kibana before any other new feature?

@briangow
Copy link

@rashidkpc I'd also like to voice that I'm very disappointed that nested aggregation support won't be added to Kibana in the near future. I recently put a lot of work into porting a database into Elasticsearch using a nested structure with the understanding that Kibana would eventually support nested aggregations. I even tested out the code developed by ppadovani and it worked for our needs. Without this support sticking with Elastic might not work for us since we require good visualization software.

@ppadovani
Copy link
Author

Update: I don't see my new branch being ready for preliminary feedback or testing for at least a month due to my work load. However I wanted to put forward what I'm doing to gather any feedback from the community as I move forward.

The basic design revolves around two things:

  1. A new query parser for the Kibana free form query field. This parser uses the standard Bison syntax definition (see the Jison project for the javascript version that I am using). The BNF I am using is based on the existing BNF we use at Homeaway for our custom query language against Elasticsearch. See my comment above for an example. I chose this approach to allow future enhancements by the community as needed.

  2. Change the getFieldMapping call in mapper.js to getMapping and process the results differently such that the nested path on fields is captured and added to the field information that Kibana stores.

When a query is typed in the parser will validate that not only have fields been correctly named, but that the values provided are valid for the field types. I.e. a date field gets a date, or a boolean was given a boolean. Additionally, nested fields will be handled automatically by the parser and the proper Elasticsearch query json will be generated instead of the simple query language that is used now.

Finally for aggregations, since the fields now contain the clues about nested paths, it will be trivial to work against my previous branch to automatically inject the nested aggregations as needed based on the fields selected.

Milestones:
1 - Get the parser functional, and generating queries
2 - Update mapper.js, and implement nested query support
3 - Implement nested aggregation support
4 - Testing/cleanup

Any feedback on this approach would be greatly appreciated. Thanks!

@chenryn
Copy link
Contributor

chenryn commented Jan 21, 2016

Kibana do support nested query by JSON style in its search box. Why need to implement a new query parser?

Just updating mapper.js to record nested_path information in .kibana index-pattern, and then the edit page of nested aggregation can check the nested_path attr to list all fields can be selected.

@ppadovani
Copy link
Author

The need for a simplified query language comes from the need for non technical product managers needing to be able to create analytic dashboards against our data. They should not have to learn the ins and outs of hand forming an elasticsearch json query.

@chenryn
Copy link
Contributor

chenryn commented Feb 22, 2016

@ppadovani is there a syntax design spec about your simplified query language?

@ppadovani
Copy link
Author

@chenryn please see the discussion here: #1084

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

Successfully merging this pull request may close these issues.