Skip to content
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

transWhere is very slow #581

Closed
acasar opened this issue May 11, 2020 · 10 comments · Fixed by #623
Closed

transWhere is very slow #581

acasar opened this issue May 11, 2020 · 10 comments · Fixed by #623

Comments

@acasar
Copy link
Contributor

acasar commented May 11, 2020

In one of my projects I've noticed that transWherecan become very slow. I don't have a solution for this issue yet (except for running 2 separate queries), so I'm posting it here in hopes that someone else might have an idea.

I have a query like this:

Product::transWhere('slug', 'roll-amore')->first();

The resulting query is:

SELECT 
    `sp_catalogue_products`.* 
FROM 
    `sp_catalogue_products` 
LEFT JOIN 
    `rainlab_translate_indexes` ON
        `sp_catalogue_products`.`id` = `rainlab_translate_indexes`.`model_id`  AND 
        `rainlab_translate_indexes`.`model_type` = 'Sp\\Catalogue\\Models\\Product' AND
        `rainlab_translate_indexes`.`locale` = 'en' 
WHERE  
    `sp_catalogue_products`.`slug` = 'roll-amore' OR ( 
        `rainlab_translate_indexes`.`item` = 'slug' AND
        `rainlab_translate_indexes`.`value` = 'roll-amore' 
    )

In my tests it can take up to 97 seconds to execute:

image

I have tested on two different database systems with similar results:

  • MariaDB 10.3.14
  • MySQL 5.7.30

There are 7k rows in sp_catalogue_products table and 8k rows in rainlab_translate_indexes, so the amount of data is not huge.

@mjauvin
Copy link
Contributor

mjauvin commented May 11, 2020

Can you check if there are indexes on all the fields used in the where/join clauses of the query?

@acasar
Copy link
Contributor Author

acasar commented May 11, 2020

Indexes seem to be ok

Indexes on rainlab_translate_indexes:

image

Indexes on sp_catalogue_products:

image

@acasar
Copy link
Contributor Author

acasar commented May 11, 2020

hm.. maybe the issue is the lack of index on the value column? But since it's typed as mediumtext index can't really be added...

image

EDIT: nope, tried changing it to varchar(511) and adding the index, but the result is still the same.

@mjauvin
Copy link
Contributor

mjauvin commented May 15, 2020

Would you mind modifying the scopeTransWhere() method with the following for testing?

public function scopeTransWhere($query, $index, $value, $locale = null, $operator = '=')
{
    if (!$locale) {
        $locale = $this->translatableContext;
    }

    return $query
        ->where(function($q) use ($index, $value) {
            $q->where($this->model->getTable().'.'.$index, $value);
            $q->orWhere(function($q) use ($index, $value) {
                $q   
                    ->where('rainlab_translate_indexes.item', $index)
                    ->where('rainlab_translate_indexes.value', $value)
                ;
            });  
        })   
        ->leftJoin('rainlab_translate_indexes', function($join) use ($locale) {
            $join
                ->on($this->model->getQualifiedKeyName(), '=', 'rainlab_translate_indexes.model_id')
                ->where('rainlab_translate_indexes.model_type', '=', get_class($this->model))
                ->where('rainlab_translate_indexes.locale', '=', $locale)
            ;
        });  
}

@acasar
Copy link
Contributor Author

acasar commented May 15, 2020

@mjauvin I replaced the code, ran it again and got the same result.

It seems to produce the same query... Am I missing something?

@mjauvin
Copy link
Contributor

mjauvin commented May 15, 2020

That's what you should expect, was just making sure.

@mterman
Copy link
Contributor

mterman commented May 22, 2020

I've had the same problem with the scopeTransWhere, it gets very slow it there is more than a couple of thousand rows. I think that there should be a different scope method for searching only in the selected locale, not both in the default locale and the selected locale.

@mjauvin
Copy link
Contributor

mjauvin commented May 22, 2020

@mterman would you be able to submit a PR for a NEW scope (we don't want to introduce a breaking change here)?

Thanks.

@mterman
Copy link
Contributor

mterman commented May 22, 2020

@mjauvin sure, I already did a quick test and it is noticeably faster. Need a better name for the scope, and test the scope in different situations.

public function scopeTransWhereNoDefault($query, $index, $value, $locale = null, $operator = '=')
{
        if (!$locale) {
            $locale = $this->translatableContext;
        }

        $query->select($this->model->getTable().'.*');
        
        if ($locale == $this->translatableDefault) {
            $query->where($this->model->getTable().'.'.$index, $operator, $value);
        } else {
            $query->where('rainlab_translate_indexes.item', $index)
                  ->where('rainlab_translate_indexes.value', $operator, $value);
        }

        $this->joinTranslateIndexesTable($query, $locale);
        return $query;
}

@mjauvin
Copy link
Contributor

mjauvin commented May 29, 2020

Did you read this section of the Documentation?

https://github.com/rainlab/translate-plugin/blob/master/README.md#indexed-attributes

@mjauvin mjauvin linked a pull request Jun 24, 2020 that will close this issue
acasar added a commit to acasar/translate-plugin that referenced this issue Nov 13, 2020
This PR is an alternative version of rainlab#586

Instead of creating a separate method that has a different behavior to the original implementation, I managed to fix ``transWhere`` itself by utilizing 2 queries instead of the slow orWhere clause. This implementation also preserves the fallback functionality.

I made some tests on my project (originally discussed here: rainlab#581) - now I have 2 queries that execute in ~ 5 ms instead of a single 60 second query. The improvement is significant.

Comments are welcome.
@mjauvin mjauvin linked a pull request Nov 15, 2020 that will close this issue
@mjauvin mjauvin removed a link to a pull request Nov 16, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants