-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
RangeQuery gt, gte, lt, lte with backward compatibility #1453
RangeQuery gt, gte, lt, lte with backward compatibility #1453
Conversation
…ters into gt, gte, lt, lte
Hi @olivere how are you doing? I'm just wondering what's the current status of this PR as I believe this solves an issue I'm currently experiencing with Currently, when using {
"query":{
"range":{
"created_date":{
"from":"now-1m/m",
"include_lower":true,
"include_upper":true,
"to":"now/m"
}
}
}
} While the documentation suggests {
"query":{
"range":{
"created_date":{
"gte":"now-1m/m",
"lt":"now/m"
}
}
}
} Many thanks, |
@hastinc I treated this PR as a "nice-to-have" mainly because the on-the-wire format still uses the old syntax even in the current version of ES. In other words: While the JSON in the request might be different (and nicer), the results that you should see in the response should be the same. If there is a reproducible bug with the current on-the-wire format, I would be happy to fix it. Feel free to file an issue. |
@olivere thanks for following up and the clear and concise explanation. |
so it's fixed now? I find use gt/lt should more faster than from/to |
it is two years ago....😓 |
So if I understand correctly, Elastic is using non-documented fields which still happen to be mapped correctly to underlying wire format because they are used in the wire format? Is there any reason we want to keep this non-documented behavior? Backwards compatibility with a very old ES version (0.90)? |
I'm closing my PR since there hasn't been activity in a long time and :
|
I'm migrating an application from Scala (Elastic4s https://github.com/sksamuel/elastic4s) to Golang (Olivere) and I've experienced the same behaviour as PR #1187
elastic.NewRangeQuery(DatePublication).Gt(xxx)
Expected : Scala, rest api, elatic documentation
VS
Got : Golang olivere
See https://www.elastic.co/guide/en/elasticsearch/reference/0.90/query-dsl-range-query.html
I have change in RangeQuery from, to, include_lower and include_upper parameters into gt, gte, lt, lte but I have kept From, To, IncludeLower and IncludeUpper for backward compatibility.
I don't know if you want some verification like theses :
🎉 Merry Christmas 🎅