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

[Saved Objects] Add support for ES query string #17475

Closed
uboness opened this issue Mar 30, 2018 · 6 comments
Closed

[Saved Objects] Add support for ES query string #17475

uboness opened this issue Mar 30, 2018 · 6 comments
Labels
stale Used to mark issues that were closed for being stale Team:Core Core services & architecture: plugins, logging, config, saved objects, http, ES client, i18n, etc

Comments

@uboness
Copy link

uboness commented Mar 30, 2018

At the moment, the saved objects client find function only supports ES's simple_query_string queries. The problem is that while this query syntax is somewhat cleaner, it is quite limited and doesn't support range queries.

Since we're in the process of introducing our own query syntax in Kibana (e.g. the simple query syntax EuiSearchBar supports and the new KQL), it would be great to add back the support query_string syntax that does support range queries, as they're super useful for navigating objects (e.g. mostly around date ranges).

It should be quite easy to add this support while maintaining bwc, by simply adding another options to the find argument indicating the syntax type. This should serve as a good stopgap solution until we figure out the idea query model that should be supported by this function (ideally, not a string, but an AST of some sort... maybe even KQL AST)

@spalger
Copy link
Contributor

spalger commented Mar 30, 2018

I have no objection to supporting range queries, but the primary reason I didn't want to support query_string was because things like name:"value" won't work because the fields are different in the index. I also don't want to encourage encoding the field naming strategy we're using in the queries people send.

@spalger
Copy link
Contributor

spalger commented Mar 30, 2018

@Bargs how would you feel about supporting the Kibana query string syntax here? Would that be a BWC nightmare?

@uboness
Copy link
Author

uboness commented Mar 30, 2018

@spalger ah, I c... yea... I get it, makes sense. So maybe query_string is not a good option. How about supporting the the EuiSearchBar.Query syntax? I mean we can extract it to be it's own thing (maybe call it EuiSearchQuery or something like that) and just have another option to support that?

we'll need to change more things to make it work (eg. make some eui services available for the server side)... but the nice thing about it is that all listing pages for the different object types already have the EUI search bar, so it's a natural fit.

how would you feel about supporting the Kibana query string syntax here? Would that be a BWC nightmare?

we can certainly support that too... but last I've heard it's still considered highly experimental and fragile at this point... in any case, once we have an option to "select" the language/syntax used, we can always add more later

@uboness uboness added the Team:Core Core services & architecture: plugins, logging, config, saved objects, http, ES client, i18n, etc label Mar 30, 2018
@epixa epixa removed the :Discovery label Mar 30, 2018
@Bargs
Copy link
Contributor

Bargs commented Apr 3, 2018

Wow, this has the potential to open a whole giant can of worms 😅

I think we should definitely use KQL here, or at the very least a subset of KQL that still compiles down to the KQL AST. KQL currently allows users to specify fields with the usual fieldName:value syntax, so it doesn't address your concerns at the moment @spalger.

@uboness this is the first I've heard of EuiSearchBar.Query and that worries me. KQL's whole raison d'être is that its AST will become the universal API for working with ES queries programmatically in Kibana. Also, over time KQL should become the default query language for most search use cases in Kibana so that users get a consistent experience across the board. If the situation really calls for them, UI elements and alternative syntax should be ok as long as they use the KQL AST under the hood.

It's true that KQL is still experimental, but 6.3 is a big step forward in terms of language refinements and additional features like autocomplete. We're making the opt-in much more visible by putting it directly in the query bar, without any need to turn it on in the advanced settings. The next step, if feedback is positive, is to make KQL the default language in the query bar.

@uboness
Copy link
Author

uboness commented Apr 4, 2018

@uboness this is the first I've heard of EuiSearchBar.Query and that worries me. KQL's whole raison d'être is that its AST will become the universal API for working with ES queries programmatically in Kibana. Also, over time KQL should become the default query language for most search use cases in Kibana so that users get a consistent experience across the board. If the situation really calls for them, UI elements and alternative syntax should be ok as long as they use the KQL AST under the hood.

I hear you... but no reason to worry :)

It's important to remember that EUI doesn't belong to Kibana... it's a general purpose UI library for all Elastic products. Cloud is a big user of it and hopefully swiftype will be too.

The KQL initiative is awesome. It truly is and I love where you guys are going with it. But also, the scope of this language is driven by the user experience in Discover. This is quite a proprietary scope that doesn't necessarily apply to what the the search bar and its query was built for.

The reason why we built the search bar came out a clear need for such a generic component (in EUI) to navigate objects - not necessarily query ES.. but provide a simple UI for totally non technical people to navigate though object - object being anything really. This navigation pops up everywhere in a well designed UI - one clear example are all the listing pages for the saved objects (the table thats shows you all available dashboards, or that of visualisations, or maybe canvases). Another good example is the new advance settings UI where the users would just like to quickly filter the view for the settings they are interested in - this isn't tied to ES at all.

When we noticed the oh so many similar use cases, it was clear that we needed a consistent generic search bar for that task. The requirement here were much (much) different than the one you have with KQL:

  • target audience - totally non technical people
  • as simple as a query language can be, but not simpler - we don't want to support too many features as it ties to the user experience in the following bullet point
  • we don't expect users to user sophisticated query language at all - if they want, they can, but we don't even want to encourage them. For this, it's important to limit the scope of such language, such that it'll be relatively easy to build additional controls on top of it to basically edit it. The search bar does exactly that.. the powerful feature in the search bar is not the query syntax itself, but the filter controls that it provides, which essentially manipulate the query.

It should be dead simple to search for objects, to filter by certain fields, to support range queries on dates, etc...

This is why the search bar was build and that's the scope of it.

Now, where I'm totally cool with a statement of "KQL is the de-facto language kibana works with". That's great... but it doesn't really clash with the search bar. If we want to turn KQL AST into that query DSL that is exposed everywhere in the Kibana APIs (e.g. saved objects API), that's great... since we're dealing with ASTs, it's just a matter of converting the EUI search bar query ast to KQL AST for the places where we actually search for saved objects. But as I said, that's not the only use case and not the only place where the search bar is needed.

As for the saved objects API itself, we had a few discussions about what should the AST be... should we keep it just a string (as it is today) but enable "flagging" different languages/syntaxes? Should it define its own AST that is really limited to the "search power" we'd like to expose by this API? Should it be KQL AST?

After a bunch of discussion, it was made clear that we should not rely on KQL just yet... maybe in the future, but not now at least. With that, we needed to move forward as well. So after discussions I had with Spencer we decided to go down this route of adding an "experimental" proprietary AST as an addition to the current API. We made it experimental (or "unsafe" as spencer puts it) that it'll be clear that this AST may change, so it's for internal use only. It doesn't break bwc in its current form and it doesn't guarantee fwc for later. It's simply a stopgap solution that enables us to move further instead of waiting for KQL to be "ready".

@rudolf
Copy link
Contributor

rudolf commented Oct 14, 2019

We've introduced KQL support to find() in #47012

@joshdover joshdover added the stale Used to mark issues that were closed for being stale label Jan 14, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
stale Used to mark issues that were closed for being stale Team:Core Core services & architecture: plugins, logging, config, saved objects, http, ES client, i18n, etc
Projects
None yet
Development

No branches or pull requests

7 participants