pmatseykanets / laravel-scout-postgres

PostgreSQL Full Text Search Engine for Laravel Scout
MIT License
160 stars 38 forks source link

Allow to customize the SQL search query with Closure v2 #21

Closed tortuetorche closed 6 years ago

tortuetorche commented 6 years ago

Notice: This pull request is a refactoring of my second pull request https://github.com/pmatseykanets/laravel-scout-postgres/pull/19 Without the need of the $bindings variable.

Use case:

App\User::getModel()->searchableUsing()->extendQuery(function ($query) {
    return $query
        ->addSelect('name')
        ->where('organization_name', 'GitHub')
        ->whereIn('role', ['admin', 'moderator'])
        ->whereYear('created_at', '>', date('Y') - 2);
});
$users= App\User::search('Peter')->raw();
pmatseykanets commented 6 years ago

Sorry I'should've explained it better how you could do it in #20. Here's an example how you'd do the same if we pass $query and $bindings into the Builder callback in #20

$results = App\Post::search('fat & (cat | rat)', function ($builder, $config, $query, $bindings) {
    // You can potentially customize the query 
    $query->selectRaw("case when author_id in (?, ?) then ? else ? end as selected_authors"); 
    $query->whereRaw('not exists (select 1 from blacklisted where author_id = posts.author_id limit 1)');

    $bindings->push(375)->push(389)->push('Yes')->push('No');

    return new ToTsQuery($builder->query, $config);
})->raw();

But again back to my point it's still very fragile and there is only so much you can customize before you break the whole query.

tortuetorche commented 6 years ago

🤔 I don't understand very well... I mean, when you write SQL or Eloquent query or code in general, you can easily break things, isn't it? And this feature isn't a default behavior of the package, it's for advanced usage only. So the responsibility to not break the query belongs to the developer. Maybe I misunderstand something, sorry I'm a not a native English speaker...

pmatseykanets commented 6 years ago

Let me try to explain it better.

I mean, when you write SQL or Eloquent query or code in general, you can easily break things, isn't it?

Yes, but when you do so all your code that produces the query lives in the "user land" (your application code).

Now in contrast Scout code in general and PostgreSQL driver's specifically is no longer your "user land" code. So the issue lays in the fact that the FTS query starts being built in the driver's code but then is being augmented in an unknown and uncontrollable for the driver way in the app code.

If in the next driver's version we'd change the way the query is structured there is a good chance it would break all the places where the callback is used for customizations although the change itself is not introducing any otherwise externally observable BCs.

Now the Scout's Builder already allows some degree of customization with simple wheres and orderBys. If we convince Taylor to add select in some form to the Builder then most of the simple customizations would be possible without the need to hook into the driver's internals.

On the same note Scout as I see it was never meant to be used to craft sophisticated custom FTS queries. If I were in a such situation I most likely wouldn't use Scout and my own driver but rather build and run this sophisticated FTS query directly in my app code so have full control over it or better yet push it to the database side (stored function, view or such) altogether if possible.

Hope this makes a little bit more sense now.

tortuetorche commented 6 years ago

Thank you for taking time to answer me, now I understand your point of view. I think the most efficient way for accomplish my goals is to fork your package and apply small changes on it. Here my fork https://github.com/tortuetorche/laravel-scout-postgres, need some docs, but it's a good start 😄