Support for Named Queries missing in NEST Query DSL?

[daflookie]

Per ES docs, since 0.90.4 each query can support a Name via the "_name" property, however, this looks to be missing in NEST. The resultant "matched_queries" string array on each hit in the response would also need to be added.

I am looking to leverage this feature in an existing project. If it indeed is not present would you be interested in a PR that adds it? The documentation states all queries support this, and for starters I'm not sure if that literally means 100% of all possible query types or not (MLT, geo related, fuzzy, etc). If you know that answer off hand great, if not I could dig around in the ES source itself to try and figure that out so we have complete coverage.

[gmarz]

Nice catch @daflookie.

It looks like this is implemented on the request side for filters:

https://github.com/elasticsearch/elasticsearch-net/blob/develop/src/Nest/DSL/Filter/FilterDescriptor.cs#L15

https://github.com/elasticsearch/elasticsearch-net/blob/develop/src/Nest/DSL/Filter/FilterDescriptor.cs#L940

but missing entirely for queries, and on the response for both.

We should do something along the same lines as this for QueryDescriptor<T>, and also add matched_queries to ISearchResponse.

A PR for this would be awesome if you're up for it 👍. Let us know if you need any help getting started or get stuck anywhere.. No need to dig through the ES source code to find all the queries that support _name...the majority of them do, if not all- at least enough to justify doing this at a base level like we're already doing for filters.

[daflookie]

Sure thing @gmarz ... I likely can send you a PR within a couple few days

[gmarz]

Hey @daflookie

Just want to touch base with you and see if you've taken a crack at this yet? We'd really like to have this for our upcoming 1.4 release and are contemplating just implementing ourselves- but don't want to crush any progress you've made if you're on the brink of opening a PR.

[daflookie]

Sorry @gmarz - I had a couple setbacks outside of getting this done, and didn't devote the kind of time I was expecting those first few days.

I haven't pushed changes to my fork, but in a nutshell the response side of things and updating the NamedFilter's integration test is totally straightforward and completed. The response side was just a couple of changes so there really is no lost work/effort IMO.

I was aiming to implement query names at a base level like you recommended, but it seems to me there is much more judicious use of base and abstract classes for Nest's filters compared to queries. Almost all the filter and filter descriptors inherit from PlainFilter and FilterBase respectively (and more importantly both of those implement IFilter). On the query side however there looks to be more touch points than I was anticipating when giving IQuery a QueryName property. So much so that I just now took a step back to contemplate the optimal approach.

If you guys want to implement yourself no prob. I'm not too close to this lib, but if it were me I'd consider refactoring it to have both PlainQuery and QueryBase implement IQuery (like filters). Eventually other query-wide properties like Boost could also find there way.

Thoughts from here?

[gmarz]

No need for an apology @daflookie :). We appreciate you opening an issue and bringing this to our attention, let alone attempting to implement it yourself 👍.

I definitely agree that our query objects could benefit from some further/better abstraction, but it's probably more of a 2.0 thing as we have a pretty strict backwards compatibility policy that we need to oblige to. In the mean time, we're better off fitting this into what we currently have in place as best as we can.

We'll go ahead and implement this - thanks again for your efforts and for opening this issue!

[daflookie]

Sounds good looking forward to seeing this released. Am I right that when this issue is closed I could leverage it with the bleeding edge packages you all publish?

[gmarz]

That is correct.

[gmarz]

Breaking this issue into two issues:

#1217 - for adding matched_queries to ISearchResponse so at least named filters will be fully implemented.

Leaving this issue open for adding support for named queries, but this will most likely be a 2.0 change since it requires heavy refactoring of the current query objects.