Query DSL support about go-elasticsearch HOT 19 CLOSED

This package is intentionally a low-level client, while olivere/elastic is a high-level client with extensions for building the requests and deserializing the responses. We are aiming for offering a more high-level API in the future, but — as I've indicated in other tickets — no sooner than a machine-readable, formal specification of the request and response bodies is available.

That is especially true for the QueryDSL — in order to be able to promise a strong compatibility contract, there needs to be a formal specification to generate the Go API from.

With that in mind, though, I've been thinking about adding some kind of helper methods for similar use cases (search definition, bulk building, ...). Something like esapi.JSONReader(), which would automatically serialize a map[string]interface{} passed to it, to lighten the load on application code.

On the other hand, I was reasonably happy when using a strings.Builder to build search definitions, eg. in the xkcd example.

from go-elasticsearch.

Yes, completely agreed that building a complicated search definition by hand is laborious... But it's still a long way to get to a place where the code can be generated, as it is now with the API endpoints and parameters...

I'll need to wrap up couple of other things first, but I'll ping on this ticket once I'm looking into the helpers.

from go-elasticsearch.

I got lost in another project, so this issue fell off my radar. The esutil.JSONReader helper has been added in the meantime, check the example:

@Youhar, will this help for working with the request bodies?

from go-elasticsearch.

Yes, that issue looks like a good one to subscribe to in this regard :)

I'll close this issue then, if it's OK. Thanks for the discussion!

from go-elasticsearch.

For people stumbling on this issue, aquasecurity/esquery uses a syntax similar to olivere/elastic and you can use it with esutil.NewJSONReader.

from go-elasticsearch.

A JSON Reader would indeed streamline the process a lot, I've built a couple search queries using a strings.Builder and once you start handling function score queries with multiple nested filters and boosts it gets a little messy (albeit fully functional if you just test the generated string).
Please let me know if I can help in any way in implementing a JSONReader, I'd be happy to 😉

from go-elasticsearch.

@karmi How is JSONReader different from json.Marshaler. I mean, yes, it is more generic in that it uses io.Writer. But is it worth it?

E.g. tools like github.com/tidwall/gjson and github.com/tidwall/sjson operate on standard interfaces/data structures used by encoding/json ([]byte in that case). So I think integration with tools would be easier if we'd rely on those defined in the standard library.

Just my 2 cents.

from go-elasticsearch.

I'm not sure I follow 100% percent the comparison to json.Marshaler. The purpose of the JSONReader is to facilitate passing custom objects like maps or structs to the APIs which accept an io.Reader, like the Index API:

sjson.Set returns a string (or bytes), so the user code would still have to wrap it in a reader. Therefore, I'm not sure what's the concrete suggestion for a different approach — can you clarify?

Also, the motivation is to make it easier to pass values to the APIs, not to decode the responses — the _examples folder contains an example using gjson.Get for that.

from go-elasticsearch.

OK. Maybe the comparison wasn't fair. One of the big failures I did with elastic was to use my own interface contract for doing serialization/deserialization instead of json.Marshaler and json.Unmarshaler. And this looked like it would go in the same direction.

from go-elasticsearch.

I see. I think this doesn't introduce any custom contract, since the core API accepts the most generic interface there is: io.Reader. This helper is really only for making the "I just need to do something quickly" use case a bit simpler, by taking eg. a map[string]string and converting it to a JSON payload wrapped inside a reader.

from go-elasticsearch.

Yes, I saw the commit a month ago and re-implemented my search request using the JSONReader. It has definitely improved both maintainability and development speed, thanks a lot.

As a side note, I decided to go with typed structs instead of map[string]interface{} for readability, and I found myself redefining generic DSL queries, for example:

// BoolQuery Elastic bool query
type BoolQuery struct {
	Bool BoolQueryParams `json:"bool"`
}

// BoolQueryParams params for an Elastic bool query
type BoolQueryParams struct {
	Must               interface{} `json:"must,omitempty"`
	Should             interface{} `json:"should,omitempty"`
	Filter             interface{} `json:"filter,omitempty"`
	MinimumShouldMatch int         `json:"minimum_should_match,omitempty"`
}

I think having those included in the project would help a lot (I didn't look into generating them from the api docs though, so maybe easier said than done).

from go-elasticsearch.

Thanks! I'm glad to hear that it's helpful. And yes, absolutely, structs make more sense here, the maps I've used were mostly for illustration.

The problem with the structs is that at the moment, we don't have any formal specification for either request or response bodies — only for the API "surface". See eg. the specification for bulk.

It's definitely something we want to have, because without a spec like that, it's not feasible to add these structs (or helper methods, etc) for typed languages, and it does complicate something like a Ruby DSL as well.

It's technically "possible" to do that by hand, but as eg. olivere/elastic or the high-level .NET client demonstrates, almost "impossible" to do, because it requires insane amounts of manual labour.

from go-elasticsearch.

I see. I'll wait for any update on more detailed request/response bodies specifications then 😉 (elastic issue #25152 right?)

from go-elasticsearch.

Hi @karmi,
I'm reopening this issue as it seems the esutil.JSONReader needs some fixing. I had this issue where writing the body using this method would send an empty request (no body).
I traced it down to this switch statement on body.(type) in the newRequest method. It seems, another case should be added for JSONReader. I opened a PR #59 on this issue, could you please take a look at it?

from go-elasticsearch.

Right, there was a bug with the implementation, but should have been fixed already in 82fc17d — at least from the testing included.

I've evaluated the adding another case (and Len() etc), just like you did, but we then figured out another way of doing it in a review. Do you have some code which fails with the current master (or 7.x etc) branches, so I can run it on my end?

from go-elasticsearch.

Ah my bad, didn't see that commit. I have failing code but on a prior version without the default case, hence my PR (I was using v6.7.1 instead of v6.8.1).
Thanks for the quick response, I'll close the PR

from go-elasticsearch.

Argh, sorry about that. Hope you don't mind the exercise :) So with a new version your code runs OK?

Basically that fix implements a io.WriterTo() interface, so it can skip the Len() etc.

from go-elasticsearch.

Yep, I just tested it and it works flawlessly after that damn go get -u

from go-elasticsearch.

Perfect, thanks for confirmation!

from go-elasticsearch.