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

Add search-exists API to check if any matching documents exist for a given query #7026

Merged
merged 1 commit into from Jul 31, 2014

Conversation

Projects
None yet
3 participants
@areek
Copy link
Contributor

commented Jul 24, 2014

Implements a new Exists API allowing users to do fast exists check on any matched documents for a given query.

This API should be faster then using the Count API as it will:

  • early terminate the search execution once any document is found to exist
  • return the response as soon as the first shard reports matched documents

closes #6995

@areek

This comment has been minimized.

Copy link
Contributor Author

commented Jul 24, 2014

The request format would be identical to that of the Count API:

curl -XGET 'http://localhost:9200/_exists' -d '
{
    "query" : {
        ...
    }
}'

Additionally one can specify preference, type, min_score and routing. Additionally filter on index(s) and type(s).

The Response would be as simple as:

{
    "exists": true,
    "_shards": {
       ...
    }
}

I have intentionally not integrated it with the _search endpoint, as yet another flag has to be introduced in the response, as hits would not be a good container for the exists flag.

@areek

This comment has been minimized.

Copy link
Contributor Author

commented Jul 24, 2014

TODO:

@areek

This comment has been minimized.

Copy link
Contributor Author

commented Jul 24, 2014

Updated PR:

  • fixed concurrency error for early termination of shard requests (thanks @kimchy)

TODO:

  • Documentation
  • Rest Spec/Tests

@areek areek self-assigned this Jul 24, 2014

@areek

This comment has been minimized.

Copy link
Contributor Author

commented Jul 28, 2014

@clintongormley would be awesome if you could take a look at the docs for the new API. The docs are quite similar to that of the count API.

TODO:

  • rest spec/tests
@clintongormley

This comment has been minimized.

Copy link
Member

commented Jul 28, 2014

Hi @areek

This looks good, but I think the REST API needs to change to be consistent. Exists requests in the REST layer are implemented as HEAD requests which don't return a body, just a 200 or a 404. (they can return other HTTP error codes if some other error occurs)

I think this should be:

HEAD /{index}/{type}/_search

And in the rest specs, it should probably be called search_exists (can you think of a better name?) to avoid confusion with the exists API.

@areek

This comment has been minimized.

Copy link
Contributor Author

commented Jul 28, 2014

@clintongormley I changed the REST API to handle HEAD, I still have support for GET and POST due to users ability to send a request body.

Regarding the naming, I was thinking hit_exists. Thoughts?

@areek

This comment has been minimized.

Copy link
Contributor Author

commented Jul 28, 2014

@clintongormley As far as I have seen, it seems like we don't have an explicit 'exists' API. I am assuming you are referring to HEAD /{index}/{type}/{id} in the GET API used to determine doc existence based on id. So I think it is ok to use _exists endpoint here. Thoughts always welcome and thanks for suggesting HTTP HEAD!

The updated REST resp/request are as follows:

curl -XHEAD -i "localhost:9200/{index|null}/{type|null}/_exists" -d '
{
  "query": ...
}
'
or 
curl -XHEAD -i "localhost:9200/{index|null}/{type|null}/_exists?q=.."

The HTTP GET and POST are also supported for having request body.

The response header is a simple:

HTTP/1.1 200 OK
Content-Type: text/plain; charset=UTF-8
Content-Length: 0

or

HTTP/1.1 404 Not Found
Content-Type: text/plain; charset=UTF-8
Content-Length: 0

The response will not have any content for any of the supported HTTP methods.

@areek areek added the review label Jul 28, 2014

@clintongormley

This comment has been minimized.

Copy link
Member

commented Jul 29, 2014

@areek Bah, I'd completely forgotten about the issue with HEAD and bodies! Apologies. And yes, by "exists" API I was referring to HEAD /{index}/{type}/{id}.

OK, so rethinking this: We already have /_search/scroll, /_search/percolate, /_search/template, wondering if this should be /_search/exists.

Also, the clients expect a body in the response for all requests except for HEAD requests, but as you've pointed out, eg the JS client won't be able to use a HEAD request here, so it looks like returning a body will be required.

Sorry for the runaround

@s1monw

This comment has been minimized.

Copy link
Contributor

commented Jul 29, 2014

aside of @clintongormley comments this looks fantastic!

@s1monw s1monw removed the review label Jul 29, 2014

@areek

This comment has been minimized.

Copy link
Contributor Author

commented Jul 29, 2014

@clintongormley I have updated the REST req/res as follows:

  • Support HTTP HEAD, GET and POST methods
    • If HTTP HEAD is used, no body is returned only the status
    • else response body looks as follows:
     {
       "exists" : true | false
     }
  • Added additional endpoint /_search/exists (/_exists can be used to as described in previous comment)
  • Updated Docs

@areek areek added the review label Jul 30, 2014

@clintongormley

View changes

docs/reference/search/exists.asciidoc Outdated
@@ -0,0 +1,97 @@
[[search-exists]]
== Exists API

This comment has been minimized.

Copy link
@clintongormley

clintongormley Jul 30, 2014

Member

Call this the "Search Exists API" to make it clear which "exists" functionality it refers to in the docs search dropdown.

This comment has been minimized.

Copy link
@areek

areek Jul 31, 2014

Author Contributor

renamed to Search Exists API.

@clintongormley

View changes

docs/reference/search/exists.asciidoc Outdated
--------------------------------------------------

NOTE: The query being sent in the body must be nested in a `query` key, same as
the <<search-search,search api>> works added[1.0.0.RC1,The query was previously the top-level object].

This comment has been minimized.

Copy link
@clintongormley

clintongormley Jul 30, 2014

Member

The query was the top-level object in "count", not "search", so I'd remove the added bit.

This comment has been minimized.

Copy link
@areek

areek Jul 31, 2014

Author Contributor

removed the added bit. Thanks for catching that!

@clintongormley

This comment has been minimized.

Copy link
Member

commented Jul 30, 2014

Hi @areek

I'm tempted to remove support for HEAD and for /_exists completely:

  • Remove HEAD because, even for clients where it works, there may be a proxy in-between the client and ES where it doesn't work, so we'll all end up implementing it as GET or POST anyway.
  • Remove /_exists because it uses up a namespace which is more explicitly described by /_search/exists. /{index}/_exists looks like it would check the existence of an index.

So I think this should be exposed only as :

GET | POST /_search/exists
GET | POST /{index}/_search/exists
GET | POST /{index}/{type}/_search/exists
@clintongormley

This comment has been minimized.

Copy link
Member

commented Jul 30, 2014

also, the response body looks good.

@areek

This comment has been minimized.

Copy link
Contributor Author

commented Jul 31, 2014

Hi @clintongormley thanks for the review. I have updated the documentation and the REST endpoints as suggested.

@clintongormley

This comment has been minimized.

Copy link
Member

commented Jul 31, 2014

LGTM

@areek

This comment has been minimized.

Copy link
Contributor Author

commented Jul 31, 2014

I think this is ready. I will commit this soon, if there are no objections.

Search Exists API: Checks if any matching documents exist for a given…
… query

Implements a new Exists API allowing users to do fast exists check on any matched documents for a given query.
This API should be faster then using the Count API as it will:
 - early terminate the search execution once any document is found to exist
 - return the response as soon as the first shard reports matched documents

closes #6995

@areek areek merged commit 1d581e6 into elastic:master Jul 31, 2014

@areek areek deleted the areek:feature/6995 branch Jul 31, 2014

@areek areek removed the review label Jul 31, 2014

@clintongormley clintongormley changed the title Exists API: check if any matching documents exist for a given query Search: Add Exists API to check if any matching documents exist for a given query Sep 10, 2014

@clintongormley clintongormley changed the title Search: Add Exists API to check if any matching documents exist for a given query Add search-exists API to check if any matching documents exist for a given query Jun 6, 2015

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.