-
Notifications
You must be signed in to change notification settings - Fork 357
Full-text improvements #947
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
Changes from 10 commits
cd31822
d7397ae
d9ea8c6
31e824f
fe60841
c976992
23dc50e
38fadc4
64a1748
1d1f042
31ad3cc
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -61,6 +61,34 @@ public function search(Builder $builder) | |
| ]; | ||
| } | ||
|
|
||
| /** | ||
| * Get the Eloquent models for the given builder. | ||
| * | ||
| * @param \Laravel\Scout\Builder $builder | ||
| * @param int|null $page | ||
| * @param int|null $perPage | ||
| * @return \Illuminate\Database\Eloquent\Collection | ||
| */ | ||
| protected function searchModels(Builder $builder, $page = null, $perPage = null) | ||
| { | ||
| return $this->buildSearchQuery($builder) | ||
| ->when(! is_null($page) && ! is_null($perPage), function ($query) use ($page, $perPage) { | ||
| $query->forPage($page, $perPage); | ||
| }) | ||
| ->when($builder->orders, function ($query) use ($builder) { | ||
| foreach ($builder->orders as $order) { | ||
| $query->orderBy($order['column'], $order['direction']); | ||
| } | ||
| }) | ||
| ->when(! $this->getFullTextColumns($builder), function ($query) use ($builder) { | ||
| $query->orderBy($builder->model->getTable().'.'.$builder->model->getScoutKeyName(), 'desc'); | ||
| }) | ||
| ->when($this->shouldOrderByRelevance($builder), function ($query) use ($builder) { | ||
| $this->orderByRelevance($builder, $query); | ||
| }) | ||
| ->get(); | ||
| } | ||
|
|
||
| /** | ||
| * Paginate the given search on the engine. | ||
| * | ||
|
|
@@ -94,6 +122,9 @@ public function paginateUsingDatabase(Builder $builder, $perPage, $pageName, $pa | |
| ->when(! $this->getFullTextColumns($builder), function ($query) use ($builder) { | ||
| $query->orderBy($builder->model->getTable().'.'.$builder->model->getScoutKeyName(), 'desc'); | ||
| }) | ||
| ->when($this->shouldOrderByRelevance($builder), function ($query) use ($builder) { | ||
| $this->orderByRelevance($builder, $query); | ||
| }) | ||
| ->paginate($perPage, ['*'], $pageName, $page); | ||
| } | ||
|
|
||
|
|
@@ -129,32 +160,10 @@ public function simplePaginateUsingDatabase(Builder $builder, $perPage, $pageNam | |
| ->when(! $this->getFullTextColumns($builder), function ($query) use ($builder) { | ||
| $query->orderBy($builder->model->getTable().'.'.$builder->model->getScoutKeyName(), 'desc'); | ||
| }) | ||
| ->simplePaginate($perPage, ['*'], $pageName, $page); | ||
| } | ||
|
|
||
| /** | ||
| * Get the Eloquent models for the given builder. | ||
| * | ||
| * @param \Laravel\Scout\Builder $builder | ||
| * @param int|null $page | ||
| * @param int|null $perPage | ||
| * @return \Illuminate\Database\Eloquent\Collection | ||
| */ | ||
| protected function searchModels(Builder $builder, $page = null, $perPage = null) | ||
| { | ||
| return $this->buildSearchQuery($builder) | ||
| ->when(! is_null($page) && ! is_null($perPage), function ($query) use ($page, $perPage) { | ||
| $query->forPage($page, $perPage); | ||
| }) | ||
| ->when($builder->orders, function ($query) use ($builder) { | ||
| foreach ($builder->orders as $order) { | ||
| $query->orderBy($order['column'], $order['direction']); | ||
| } | ||
| }) | ||
| ->when(! $this->getFullTextColumns($builder), function ($query) use ($builder) { | ||
| $query->orderBy($builder->model->getTable().'.'.$builder->model->getScoutKeyName(), 'desc'); | ||
| ->when($this->shouldOrderByRelevance($builder), function ($query) use ($builder) { | ||
| $this->orderByRelevance($builder, $query); | ||
| }) | ||
| ->get(); | ||
| ->simplePaginate($perPage, ['*'], $pageName, $page); | ||
| } | ||
|
|
||
| /** | ||
|
|
@@ -196,9 +205,11 @@ protected function initializeSearchQuery(Builder $builder, array $columns, array | |
| return $query; | ||
| } | ||
|
|
||
| return $query->where(function ($query) use ($builder, $columns, $prefixColumns, $fullTextColumns) { | ||
| $connectionType = $builder->model->getConnection()->getDriverName(); | ||
| [$connectionType] = [ | ||
| $builder->model->getConnection()->getDriverName(), | ||
| ]; | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Sorry to bother you again but why are you converting the model connection type to an array with one element just to retrieve the first and only element right after? Wouldn't it be easier to assign the connection type directly? $connectionType = $builder->modelConnectionType();
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah I was assigning multiple variables here in an earlier version. |
||
|
|
||
| return $query->where(function ($query) use ($connectionType, $builder, $columns, $prefixColumns, $fullTextColumns) { | ||
| $canSearchPrimaryKey = ctype_digit($builder->query) && | ||
| in_array($builder->model->getKeyType(), ['int', 'integer']) && | ||
| ($connectionType != 'pgsql' || $builder->query <= PHP_INT_MAX) && | ||
|
|
@@ -212,11 +223,7 @@ protected function initializeSearchQuery(Builder $builder, array $columns, array | |
|
|
||
| foreach ($columns as $column) { | ||
| if (in_array($column, $fullTextColumns)) { | ||
| $query->orWhereFullText( | ||
| $builder->model->qualifyColumn($column), | ||
| $builder->query, | ||
| $this->getFullTextOptions($builder) | ||
| ); | ||
| continue; | ||
| } else { | ||
| if ($canSearchPrimaryKey && $column === $builder->model->getScoutKeyName()) { | ||
| continue; | ||
|
|
@@ -229,9 +236,60 @@ protected function initializeSearchQuery(Builder $builder, array $columns, array | |
| ); | ||
| } | ||
| } | ||
|
|
||
| if (count($fullTextColumns) > 0) { | ||
| $query->orWhereFullText( | ||
| array_map(fn ($column) => $builder->model->qualifyColumn($column), $fullTextColumns), | ||
| $builder->query, | ||
| $this->getFullTextOptions($builder) | ||
| ); | ||
| } | ||
| }); | ||
| } | ||
|
|
||
| /** | ||
| * Determine if the query should be ordered by relevance. | ||
| */ | ||
| protected function shouldOrderByRelevance(Builder $builder): bool | ||
| { | ||
| // MySQL orders by relevance by default, so we will only order by relevance on | ||
| // Postgres with no developer-defined orders. If there is developer defined | ||
| // order by clauses we will let those take precedence over the relevance. | ||
| return $builder->modelConnectionType() === 'pgsql' && | ||
| count($this->getFullTextColumns($builder)) > 0 && | ||
| empty($builder->orders); | ||
| } | ||
|
|
||
| /** | ||
| * Add an "order by" clause that orders by relevance (Postgres only). | ||
| * | ||
| * @param \Laravel\Scout\Builder $builder | ||
| * @param \Illuminate\Database\Eloquent\Builder $query | ||
| * @return \Illuminate\Database\Eloquent\Builder | ||
| */ | ||
| protected function orderByRelevance(Builder $builder, $query) | ||
| { | ||
| $fullTextColumns = $this->getFullTextColumns($builder); | ||
|
|
||
| $language = $this->getFullTextOptions($builder)['language'] ?? 'english'; | ||
|
|
||
| $vectors = collect($fullTextColumns)->map(function ($column) use ($builder, $language) { | ||
| return sprintf("to_tsvector('%s', %s)", $language, $builder->model->qualifyColumn($column)); | ||
| })->implode(' || '); | ||
|
|
||
| return $query->orderByRaw( | ||
| sprintf( | ||
| 'ts_rank('.$vectors.', %s(?)) desc', | ||
| match ($this->getFullTextOptions($builder)['mode'] ?? 'plainto_tsquery') { | ||
| 'phrase' => 'phraseto_tsquery', | ||
| 'websearch' => 'websearch_to_tsquery', | ||
| default => 'plainto_tsquery', | ||
| }, | ||
| ), | ||
| [$builder->query] | ||
| ); | ||
| } | ||
|
|
||
| /** | ||
| * Add additional, developer defined constraints to the search query. | ||
| * | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shouldn't this be
$builder->modelConnectionType()?