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

Multiple fields provided to queries return results for the last one only #19547

Closed
einorler opened this issue Jul 22, 2016 · 5 comments

Comments

Projects
None yet
4 participants
@einorler
Copy link

commented Jul 22, 2016

Elasticsearch version: 2.3.4

JVM version: 1.8

OS version: OSX 10.11

Description of the problem including expected versus actual behavior:

There is a possibility to provide multiple fields to queries like so:

{
  "query": {
    "range": {
      "age": {
        "gte": 30,
        "lte": 40
      },
      "price": {
        "gte": 10,
        "lte": 30
      }
    }
  }
}

Here I provide two fields to a single query and elasticsearch provides legitimate results for such a query. However, the results returned always only include hits from the last field request, that is, if I were to execute the query given above, I would only get results from the price field. Moreover I could not find any mentions of such behaviour in the official docs.
Analogous behaviour was observed with prefix, regexp, geo_distance queries and more.
Logically this should either provide the hits for both provided fields or not work at all.

@javanna

This comment has been minimized.

Copy link
Member

commented Jul 22, 2016

The range query supports one field, like most of the elasticsearch queries. If you need to execute two range queries you need to combine them using a bool query and two must clauses that contain one range query each.

I don't see the range query supporting multiple fields in the future. What we could do though is throw an error instead of accepting such a query, which is effectively malformed. Otherwise we make users think that we support this syntax although we don't. I am a bit on the fence about this though cause that would mean adding this "is field already set" type of check potentially in a lot of places. The "keep the last one" behaviour comes with the way we pull parse json and this same situation can happen in many other places in our codebase.

@jpountz

This comment has been minimized.

Copy link
Contributor

commented Jul 22, 2016

Maybe there are things to do to make it less likely to have this error in the future (we already had a lot of similar bugs reported, and there will probably be other ones), but I think it is important to fail on bad inputs.

@clintongormley

This comment has been minimized.

Copy link
Member

commented Jul 27, 2016

This query doesn't throw an exception in 5.0, and it should.

@javanna

This comment has been minimized.

Copy link
Member

commented Jul 27, 2016

This query doesn't throw an exception in 5.0, and it should.

Yes it should. But we have this problem in each single query of our DSL. We can fix it here and forget about all the other places where we have this problem, or maybe see what we can do to fix it in more places (preferably everywhere). I have this specific fix in one of my branches. I just felt bad about fixing this single case and wondered if we could do something better other than going through each single query one by one. That is why I had marked discuss, sorry for the confusion.

@javanna javanna self-assigned this Jul 27, 2016

@clintongormley

This comment has been minimized.

Copy link
Member

commented Jul 27, 2016

+1 for a more comprehensive fix

@javanna javanna removed the help wanted label Jul 27, 2016

javanna added a commit to javanna/elasticsearch that referenced this issue Aug 5, 2016

Throw parsing error if range query contains multiple fields
Range Query, like many other queries, used to parse when the query refers to multiple fields and the last one would win. We rather throw an exception now instead.

Closes elastic#19547
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.