While seeing why #1457256: Enable Lite Pager On Project Issues Advanced Search wasn't helping, I found out that search_api_views needs to support pager->use_count_query(). Patches coming up.

CommentFileSizeAuthor
#4 2135363-4--skip_count_query.patch3 KBdrunken monkey
#1 2135363-search_api.diff1.5 KBdrumm
#1 2135363-search_api_db-no-test.diff723 bytesdrumm

Comments

drumm’s picture

drumm
he/him
Location NY, US
commented
Status: Active » Needs review
StatusFileSize
new 2135363-search_api_db-no-test.diff723 bytes
new 2135363-search_api.diff1.5 KB

In 2135363-search_api.diff:

  • The first hunk makes the option available to Search API services. It may be convenient to default to TRUE for non-Views-related queries.
  • The second only does some pager-related things if use_count_query() is TRUE. This leaves the current page number correct when using views_litepager.

In 2135363-search_api_db-no-test.diff

This uses the option made available by the first hunk. Note it checks with !== FALSE because there isn't a default value. I'm guessing the following logic may need to be rearranged instead of the $results['result count'] = TRUE; hack.

Status: Needs review » Needs work

The last submitted patch, 1: 2135363-search_api_db-no-test.diff, failed testing.

drumm’s picture

drumm
he/him
Location NY, US
commented
Status: Needs work » Needs review
Issue tags: +Drupal.org 7.1
drunken monkey’s picture

drunken monkey
he/him
Primary language German
Location Vienna, Austria
commented
Title: Support pager->use_count_query() » Add support for Views' use_count_query() method
Category: Task » Feature request
Priority: Major » Normal
StatusFileSize
new 2135363-4--skip_count_query.patch3 KB

Note it checks with !== FALSE because there isn't a default value.

SearchApiQueryInterface::getOption has a $default parameter that does just that.
An optional setting defaulting to TRUE is always a bit awkward, though. Maybe we should make it skip count query instead? (The use of underscores vs. spaces in option names sadly isn't really unified, but I think we should stick to a non-namespaced option name without spaces here.) Or, better still, skip result count, since that's less database-centric.
In any case, the option will have to be documented in the query interface.

Patch attached that adds these two changes.

For the database module, I've created a separate issue (#2135595: Add support for the new "skip result count" option). (But by the way, do-not-test is the suffix for skipping the test bot for a patch file.)

drumm’s picture

drumm
he/him
Location NY, US
commented
Status: Needs review » Reviewed & tested by the community

Looks good.

drunken monkey’s picture

drunken monkey
he/him
Primary language German
Location Vienna, Austria
commented
Status: Reviewed & tested by the community » Fixed

Good to hear. Committed.
Thanks again!

drunken monkey’s picture

drunken monkey
he/him
Primary language German
Location Vienna, Austria
commented

Didn't look good at all, actually:

+++ b/contrib/search_api_views/includes/query.inc
@@ -241,6 +241,7 @@ class SearchApiViewsQuery extends views_plugin_query {
+    $this->query->setOption('skip result count', $this->pager->use_count_query());

I'm pretty sure there is a negation missing here, so the logic is exactly the wrong way round.
Committed the trivial fix, should work now in dev.

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.