kuzzleio / kuzzle

Open-source Back-end, self-hostable & ready to use - Real-time, storage, advanced search - Web, Apps, Mobile, IoT -
https://kuzzle.io
Apache License 2.0
1.43k stars 123 forks source link

Filters are not analyzed on advanced search using REST api #342

Closed stafyniaksacha closed 8 years ago

stafyniaksacha commented 8 years ago

Steps to reproduce :

Create 2 documents

PUT http://localhost:7511/api/1.0/index/collection/test

{
    "body": {
        "foo": "bar"
    }
}

PUT http://localhost:7511/api/1.0/index/collection/test2

{
    "body": {
        "foo": "baz"
    }
}

Search with a term filter

POST http://localhost:7511/api/1.0/index/collection/_search

{
    "query": {
        "bool": {
            "should": [
                {
                    "term": {
                        "foo": "bar"
                    }
                }
            ]
        }
    }
}

Then we have a response with all documents insted of only have test document :

"result": {
    "_shards": {
      "failed": 0,
      "successful": 5,
      "total": 5
    },
    "hits": [
      {
        "_id": "test",
        "_index": "index",
        "_score": 1,
        "_source": {
          "foo": "baz"
        },
        "_type": "collection"
      },
      {
        "_id": "test1",
        "_index": "index",
        "_score": 1,
        "_source": {
          "foo": "bar"
        },
        "_type": "collection"
      }
    ]
}
scottinet commented 8 years ago

Did you try with ES REST client directly to check that this behavior comes from Kuzzle and not from ES?

The resulting low score makes me think that this is a normal behavior with a should query: ES having a "more-matches-is-better" approach, and since there are no other documents on the collection...

ballinette commented 8 years ago

According to the doc, you the query should be passed within a "body" element in the request body: http://kuzzle.io/api-reference/?rest#search

So your search request should be like that:

{
  "body": {
    "query": {
      "bool": {
        "should": [
          {
            "term": {
              "foo": "bar"
            }
          }
        ]
      }
    }
  }
}

Anyway, with most other query types, the "body" element is optional and can be ignored. For example, a filtered search query works well with the following format:

{
  "filter": {
    "term": { "foo": "baz"}
  }
}

To make it work with a "query" object at request's root, we have a workaround for count action and we need to use it also in search action (or maybe directly within cleanData if it does not makes regressions with other actions?) see https://github.com/kuzzleio/kuzzle/blob/develop/lib/services/elasticsearch.js#L166:

/*
  ElasticSearch DSL is supposed to accept a 'query' object in the main part of the message.
  Problem is: the 'count' action only accepts a 'body' object, and any query passed to it is ignored.
  So, in order to suppress this discrepancy, if a count action is called without a body but with a query,
  we embed the query object in a body one.
*/
if (_.isEmpty(data.body)) {
  if (!_.isEmpty(data.query)) {
    data.body = { query: data.query };
    delete data.query;
  } else {
    delete data.body;
  }
}
ballinette commented 8 years ago

Reading the code it could be better to revert this commit: https://github.com/kuzzleio/kuzzle/commit/4138c780646e05c0d2fd0849a8f1d8cc33f15160 and use this code instead in the RequestObject:

if (object.body !== undefined) {
  this.data.body = object.body;
}
else if (object.query !== undefined) {
  this.data.body = object;
}
else {
  this.data.body = additionalData.body || additionalData;
}

and then, the above code in the elasticsearch service would be useless

(To be tested if there makes no regression elsewere in the code)

stafyniaksacha commented 8 years ago

Resolved