Sylius search #1784
Sylius search #1784
Conversation
| @@ -72,3 +72,46 @@ swiftmailer: | |||
| username: %sylius.mailer.user% | |||
| password: %sylius.mailer.password% | |||
| spool: { type: memory } | |||
|
|
|||
| sylius_search: | |||
There was a problem hiding this comment.
This should land in sylius.yml in CoreBundle.
There was a problem hiding this comment.
This will be fully editable by the user, do we want this to live within a bundle?
There was a problem hiding this comment.
As you can see everything that is related to sylius app lands there, so this should be there too.
|
Thanks @stloyd for the very quick initial round of feedback. We'll be working on and discussing all of your comments today and update soon. |
|
First round of fixes applied. I didn't commit the logging of the search term because I'm still working on this. Currently I would like to keep the mysql for small websites and use a specific index or type on elasticsearch for larger ones. |
| @@ -113,8 +120,6 @@ public function historyAction(Request $request) | |||
| * Render product filter form. | |||
| * | |||
| * @param Request $request | |||
| * | |||
| * @return Response | |||
…asticsearch finder
|
Hello @pjedrzejewski Rebased to the latest. |
|
Nice, I will review and make this top priority merge to avoid further conflicts. Thank you! |
|
The decrease in the quality code is quite significant (0.2 points) via https://scrutinizer-ci.com/g/Sylius/Sylius/inspections/35b78402-7517-4e64-ae64-f56189519096, we should look into refactoring this few classes... But after merge, review in progress. (I'm trying to understand the internals) |
|
The mysql part is definitely not trivial. Was challenging to achieve the desired behaviour. The elastic search one can be simplified as well since they changed the way the filter is applied on aggregations. Let me know if you have any specific blind spots. |
|
@agounaris Yeah, I think I will have couple of questions. :) First one would be - what do you think about the performance of this MATCH AGAINST dql function? Have you performed any tests? |
|
I don't have exact figures but in the process I saw that like was better only in the case of a single word. If you had a phrase or multiple terms the solution could have been really complicated and slow. In any case mysql imo should be used by smaller websites with small dataset. It provides an out-of-the-box search but it cannot perform great. |
|
Do you mean performance of the doctrine dql implementation of MATCH AGAINST? Or at the mysql-level of using it? From very early on we realised that MATCH AGAINST is the best way to perform anything close to a good search engine comparison efficiently. Compared to LIKE it is massively faster as soon as you start to compare multiple fields (which of course you want to for an ecom search facility). We don't have exact numbers because it wasn't even a contest. However, in order to use MATCH AGAINST we need a FULL TEXT index, which is not available on INNODB tables before MySQL 5.6. Considering the ORM search was supposed to be the base search, it wasn't viable to put limitations on using it. Therefore we effectively 'index' all of the searchable content (along 'tags' / facets) into a MyISAM table so that it can be efficiently and effectively searched. This does mean duplication along with a slight overhead of indexing content when it changes with a listener, but we believe this is the best approach and mimics how indexing to a real search engine works so allows for a consistent pattern and code structure. Having said that, we've been using ElasticSearch so haven't been testing ORM in anger regularly, but it performed well during implementation. The real complexity comes with the facet calculation. In order to find out which options apply to a particular facet, you need to consider the filters applied on the result-set from every other facet. This wasn't the easiest concept to explain so if you need further clarification then please ask. It was my design but not my implementation so it was harder for @agounaris to build it. |
|
@pjedrzejewski hows the review going? 👻 |
| display_name: 'Available prices' | ||
| type: range | ||
| values: | ||
| - { from: 0, to: 2000} |
There was a problem hiding this comment.
@agounaris Is it possible to override these price values from main config.yml? It seems these values always shows up in the search page along with the values defined in applications config.yml.
Is it possible to define how multiple filter on same property will be applied? When I selected multiple price range it returned zero result. What I am asking for is ability to use should instead of must in bool filter for price.
There was a problem hiding this comment.
Hello @saidul
If you override the sylius.yml you can ofc add any values you would like, dunno if this is the best practice though. Do you use sylius as external library?
You are correct I will change the must to should. I'm not sure if it should be configurable though.
There was a problem hiding this comment.
@agounaris Yeah, I use Sylius as external library. The situation is that, range filters from price is always included from CoreBundle/Resources/config/app/sylius.yml I can add more values in app/config/config.yml but I can not remove or change already included price ranges.
Thanks for the clause fix 👍 😄
|
Ok, there is a lot of stuff to do with this in terms of architecture. :) But I will do a final test today and if it's OK, it will go to master! :) I will also make a list of things to refactor! |
| $results = array(); | ||
| foreach ($resultSetFromFulltextSearch as $model=>$ids) { | ||
|
|
||
| $queryBuilder = $this->em->createQueryBuilder(); |
There was a problem hiding this comment.
@agounaris I think we should use a configurable repository method call to get the query builder instead of creating it from entity manager.
|
Boom... I will try to list things to refactor/rethink, but THANK YOU GUYS!!! :) Awesome work! |
|
:) |
Hello Guys,
This is my initial pull request for the Sylius search feature as outlined on #1583
Within SearchBundle there is a README file with a few instructions on how to use the bundle. This explains the notion of the finder and how it can be used optionally with facets for different situations such as a front-end category page, the main search results page or through the backend interface.
At this time I have implemented support for small applications using ORM (mysql actually) and larger ones with ElasticSearch. It should be easy to add more engines in future.
There are a few TODO comments in the code mentioning our decisions and what can be changed in the future, but I'm hoping for some feedback and guidance first.
Videos created for demo purposes:
http://youtu.be/mACQeN1UNzQ
http://youtu.be/XBfxdrlQyzM
Regarding Behat tests there are 2 modes, one for mysql and one for elastic search. Keep that in mind when you want to run them.