Add `search_after` parameter in the SearchAPI #16125

jimczi · 2016-01-20T14:42:11Z

Pagination of results can be done by using the from and size parameters but it can be very costly for indices with more than one shard if the from parameter is big. The search_after parameter has been added to address this problem, it provides a way to efficiently paginate from one page to the next. This parameter accepts an array of sort values, those values are then used by the searcher to sort the top hits from the first document that is equal to the sort values.
This parameter must be used in conjunction with the sort parameter, it must contain exactly the same number of values than the number of fields to sort on.

NOTE: A field with one unique value per document should be used as the last element of the sort specification. Otherwise the sort order for documents that have the same sort values would be undefined. The recommended way is to use the field _uid which is certain to contain one unique value for each document.

Relates to #8192

jimczi · 2016-01-20T15:22:46Z

@jpountz this is a work in progress, I still need to write more tests to cover cases with scripts, geo and nested sort. I called it "search_from" rather than "search_after" because searching from somewhere (rather than search after) is also a use case and also because "search_from" with from=1 is equivalent to a search after.

rjernst · 2016-01-20T19:36:54Z

search_from is a very confusing name, given the existing from parameter.

jimczi · 2016-01-20T21:28:03Z

@rjernst, search_after with a parameter ìnclude_top to indicate if the top document (the one matching exactly the search_from values) must be included in the result ?

"search_after": {
    "values": ["tweet#3465"]
    "include_top": true|false
}

jpountz · 2016-01-20T22:06:09Z

core/src/main/java/org/elasticsearch/search/internal/DefaultSearchContext.java

@@ -19,13 +19,8 @@

 package org.elasticsearch.search.internal;

+import org.apache.lucene.search.*;


beware that the build now bans wildcard imports

jpountz · 2016-01-20T22:44:47Z

I would need to dive in a bit more but this looks rather good for a WIP! I agree with Ryan search_from would be confusing. Maybe we would need @clintongormley 's naming skills here. Something else I was wondering about is whether we want it as a top-level element or not (thinking of putting it under sort right now).

The recommended way is to use the field _uuid which is certain to contain one unique value for each document.

This recommendation is problematic given that _uid does not have doc values and that adding doc values to _uid is controversial: #11887. I'm not sure what to say besides requiring that the set of provided sort values should uniquely identify a document given that documents that compare equal will be skipped?

clintongormley · 2016-01-21T14:18:25Z

search_from is a very confusing name, given the existing from parameter.

I called it "search_from" rather than "search_after" because searching from somewhere (rather than search after) is also a use case and also because "search_from" with from=1 is equivalent to a search after.

Hmmm I don't like the interaction between search_from and from. These settings should be exclusive. I think that, if you want to include the previous last result as the first result in the next page, then you should just choose the values from the previous second-last doc, instead of the last doc.

Something else I was wondering about is whether we want it as a top-level element or not (thinking of putting it under sort right now).

sort is pretty overloaded already - I don't think it is the right place to put it.

from today means "how many docs should I skip before i start returning results, and defaults to 0.

Two possibilities:

support "from": ["val1","val2"] to mean "start returning results after these sort values", or
"search_after": ["val1", "val2"] and prohibit the use of from in combination with search_after

I'm leaning towards the second option

jimczi · 2016-01-21T14:54:42Z

@clintongormley I agree, I'll change the PR with the second option (search_after and prohibit from+ search_after combination).

jimczi · 2016-01-26T11:11:37Z

@jpountz @clintongormley I renamed search_fromto search_after and prohibited the usage of from!= 0 when search_after is used.

This recommendation is problematic given that _uid does not have doc values and that adding doc values to _uid is controversial: #11887. I'm not sure what to say besides requiring that the set of provided sort values should uniquely identify a document given that documents that compare equal will be skipped?

What do we recommend then ? It's not only for search_after but for pagination in general. For a pure search use case we should have an easy way to provide a deterministic sort. IMO _uid is the simplest and the safest solution. Now, should we activate the doc_values per default on this field ? Maybe not but I don't see why it could not be activated by the user. Bottom line is that the sort would work on _uid even if the doc_values are not activated, are we planning to remove the ability to sort on a field without doc_values ?

clintongormley · 2016-01-26T12:42:36Z

We recommend using _uid as a tiebreaker (and really there is no other readily available field that can be used), plus the random_score function in the function score query relies on the _uid... I think that using the _uid is the best advice we can give at the moment, and rather than blocking this feature, we should see if we can make progress on #11887

jpountz · 2016-01-26T13:31:45Z

core/src/main/java/org/elasticsearch/search/query/QueryPhase.java

-                    lastEmittedDoc = null;
+                    if (searchContext.searchAfter() != null) {
+                        after = searchContext.searchAfter();
+                    }


do we need the outer if statement or could we just do after = searchContext.searchAfter();?

Yes sure, the if is useless.

jpountz · 2016-01-26T14:51:10Z

I left some comments but it looks good overall!

jimczi · 2016-01-26T17:05:34Z

@jpountz thanks for the review, I think I've covered all your comments.

jpountz · 2016-01-26T17:10:25Z

rest-api-spec/src/main/resources/rest-api-spec/api/search.json

+        },
+        "search_after": {
+          "type" : "list",
+          "description" : "A comma-separated list of sort values that indicates where the sort of the top hits should start"


s/A comma-separated list/An array/ ?

Actually, as a query string param, it'd have to be a comma-separated list of sort values. Honestly, I'm not sure we can reliably pass these values in via query string parameters, eg it is possible for a sort value to be null, but that would become the string "null" as a query string param.

@clintongormley right but this PR does not handle null values. The sort values in the response from the previous request should contain the default value instead of null. I'll add a note in the documentation about this limitation.

@jimferenczi the default value returned in the REST API for a missing string value is null

@jimferenczi I had missed that, why can't it deal with nulls?

jpountz · 2016-01-26T17:10:53Z

LGTM

The search_after parameter provides a way to efficiently paginate from one page to the next. This parameter accepts an array of sort values, those values are then used by the searcher to sort the top hits from the first document that is greater to the sort values. This parameter must be used in conjunction with the sort parameter, it must contain exactly the same number of values than the number of fields to sort on. NOTE: A field with one unique value per document should be used as the last element of the sort specification. Otherwise the sort order for documents that have the same sort values would be undefined. The recommended way is to use the field `_uuid` which is certain to contain one unique value for each document. Fixes #8192

Add `search_after` parameter in the SearchAPI

jimczi added >feature review discuss :Search/Search Search-related issues that do not fall into other categories v5.0.0-alpha1 labels Jan 20, 2016

jpountz reviewed Jan 20, 2016
View reviewed changes

jimczi changed the title ~~[WIP] Initial implementation of the search_from parameter in the Search API.~~ Add search_after parameter in the SearchAPI Jan 26, 2016

jimczi removed the discuss label Jan 26, 2016

jpountz reviewed Jan 26, 2016
View reviewed changes

jimczi added a commit that referenced this pull request Jan 27, 2016

Merge pull request #16125 from jimferenczi/search_from

9160095

Add `search_after` parameter in the SearchAPI

jimczi merged commit 9160095 into elastic:master Jan 27, 2016

jimczi deleted the search_from branch January 27, 2016 10:09

clintongormley added the release highlight label Jan 27, 2016

jimczi removed the review label Jan 27, 2016

clintongormley mentioned this pull request Feb 13, 2016

Paginate from _id #15799

Closed

rashidkpc mentioned this pull request Feb 29, 2016

Extract log event context elastic/kibana#275

Closed

		@@ -19,13 +19,8 @@

		package org.elasticsearch.search.internal;

		import org.apache.lucene.search.*;

Add search_after parameter in the SearchAPI #16125

Add search_after parameter in the SearchAPI #16125

Uh oh!

Conversation

jimczi commented Jan 20, 2016

Uh oh!

jimczi commented Jan 20, 2016

Uh oh!

rjernst commented Jan 20, 2016

Uh oh!

jimczi commented Jan 20, 2016

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jpountz commented Jan 20, 2016

Uh oh!

clintongormley commented Jan 21, 2016

Uh oh!

jimczi commented Jan 21, 2016

Uh oh!

jimczi commented Jan 26, 2016

Uh oh!

clintongormley commented Jan 26, 2016

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jpountz commented Jan 26, 2016

Uh oh!

jimczi commented Jan 26, 2016

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jpountz commented Jan 26, 2016

Uh oh!

Uh oh!

Add `search_after` parameter in the SearchAPI #16125

Add `search_after` parameter in the SearchAPI #16125