laravel / scout

Laravel Scout provides a driver based solution to searching your Eloquent models.
https://laravel.com/docs/scout
MIT License
1.57k stars 336 forks source link

[Typesense] Using `query` together with `paginate` only yields the first page #850

Closed thannaske closed 4 months ago

thannaske commented 4 months ago

Scout Version

10.10.1

Scout Driver

Typesense

Laravel Version

10.48.16

PHP Version

8.3

Database Driver & Version

No response

SDK Version

Typesense PHP 4.9.3

Meilisearch CLI Version

No response

Description

Summary

When searching a model using Scout with the Typesense driver, using the query() method in combination with the paginate() method causes the pagination to return only the first n results (depending on the max items per page).

Example with query() method

In my concrete implementation, I'm searching through the full-text of messages. When displaying them, I need several relationships to be eagerly loaded in order to avoid n+1 query issues. According to the documentation, the query() method can used for such things:

$messages = Message::search($query)
    ->options(['query_by' => 'content'])
    ->whereIn('organisation_id', $organisations)
    ->latest()
    ->query(function (Builder $q) {
         // Eager load relationships we are using when displaying the search results
         $q->with(['ticket', 'ticket.participatingClients', 'ticket.participatingAgents', 'ticket.organisation']);
    })
    ->paginate(perPage: 10, pageName: 'messages')
    ->appends(['query' => $query]);

As you can see, we're searching by the content field in Typesense, applying some filtering and then applying some eager loading on all models found by the Scout search before paginating the results.

The following code above leads to the following result (shortened for readability):

Illuminate\Pagination\LengthAwarePaginator {#2287
  #items: Illuminate\Database\Eloquent\Collection {#2167
    #items: array:10 [
      1 => App\Models\Message { ... }
      2 => App\Models\Message { ... }
      3 => App\Models\Message { ... }
      4 => App\Models\Message { ... }
      5 => App\Models\Message { ... }
      6 => App\Models\Message { ... }
      7 => App\Models\Message { ... }
      8 => App\Models\Message { ... }
      9 => App\Models\Message { ... }
      10 => App\Models\Message { ... }
    ]
    #escapeWhenCastingToString: false
  }
  #perPage: 10
  #currentPage: 1
  #path: "https://my-application.test/client/ticket/search"
  #query: array:1 [
    "query" => "hello"
  ]
  #fragment: null
  #pageName: "messages"
  +onEachSide: 3
  #options: array:2 [
    "path" => "https://my-application.test/client/ticket/search"
    "pageName" => "messages"
  ]
  #total: 10
  #lastPage: 1
}

A LengthAwarePaginator is correctly being returned, and it shows a total of 10 results.

Example without query() method

Now I'm commenting out the query method but leaving the rest of the code completely untouched:

$messages = Message::search($query)
    ->options(['query_by' => 'content'])
    ->whereIn('organisation_id', $organisations)
    ->latest()
    // ->query(function (Builder $q) {
    //      // Eager load relationships we are using when displaying the search results
    //      $q->with(['ticket', 'ticket.participatingClients', 'ticket.participatingAgents', 'ticket.organisation']);
    // })
    ->paginate(perPage: 10, pageName: 'messages')
    ->appends(['query' => $query]);

This now yields the following result:

Illuminate\Pagination\LengthAwarePaginator {#2287
  #items: Illuminate\Database\Eloquent\Collection {#2167
    #items: array:10 [
      1 => App\Models\Message { ... }
      2 => App\Models\Message { ... }
      3 => App\Models\Message { ... }
      4 => App\Models\Message { ... }
      5 => App\Models\Message { ... }
      6 => App\Models\Message { ... }
      7 => App\Models\Message { ... }
      8 => App\Models\Message { ... }
      9 => App\Models\Message { ... }
      10 => App\Models\Message { ... }
    ]
    #escapeWhenCastingToString: false
  }
  #perPage: 10
  #currentPage: 1
  #path: "https://my-application.test/client/ticket/search"
  #query: array:1 [
    "query" => "hello"
  ]
  #fragment: null
  #pageName: "messages"
  +onEachSide: 3
  #options: array:2 [
    "path" => "https://my-application.test/client/ticket/search"
    "pageName" => "messages"
  ]
  #total: 176
  #lastPage: 18
}

The exact same search query now yields 176 total results with a total of 18 paginated pages which matches my test dataset. The only difference is now that no eager loading can be performed as the query() method has been commented out.

Steps To Reproduce

  1. Create a Laravel project using Laravel Scout with the Typesense Driver and spin-up a local Typesense instance for testing
  2. Create a new model Message with a content attribute and create and save several instances with similar or same content
  3. Create a new model Author to have a relationship that can be eagerly loaded
  4. Index the model with Laravel Scout so it can be searched
  5. Use the code examples above to alternate between eager loading a relationship using the query() method and compare the results
driesvints commented 4 months ago

It's not possible to combine query and paginate in a reliable with Scout. Please see many previous issues around this: https://github.com/laravel/scout/issues?q=is%3Aissue+query+paginate+is%3Aclosed

thannaske commented 4 months ago

If there are that many issues around that topic, it's clearly missing in the documentation.

tharropoulos commented 3 months ago

Please take a look at here to see if it's any help

igoralentyev commented 2 months ago

If there are that many issues around that topic, it's clearly missing in the documentation.

agreed 1000%. If people still comes and complain about the same thing, maybe its good idea to just add one line in the documentation like "Note that you could not combine query() with paginate()"

tharropoulos commented 2 months ago

If there are that many issues around that topic, it's clearly missing in the documentation.

agreed 1000%. If people still comes and complain about the same thing, maybe its good idea to just add one line in the documentation like "Note that you could not combine query() with paginate()"

The same question was asked on #825 and resolved accordingly. If your case seems to be different and you're still encountering errors regarding pagination, please provide a reproducible example so we can take a deeper look

igoralentyev commented 2 months ago

The same question was asked on #825 and resolved accordingly. If your case seems to be different and you're still encountering errors regarding pagination, please provide a reproducible example so we can take a deeper look

Unfortunately, the solution doesn't work in my case. I checked src/Engines/TypesenseEngine.php, everything is the same as in PR, but the problem remains.

tharropoulos commented 2 months ago

The same question was asked on #825 and resolved accordingly. If your case seems to be different and you're still encountering errors regarding pagination, please provide a reproducible example so we can take a deeper look

Unfortunately, the solution doesn't work in my case. I checked src/Engines/TypesenseEngine.php, everything is the same as in PR, but the problem remains.

Could you open up a new Issue with an example so we can take a deeper look into it?