Skip to content

#25199 Remove quotes when not necessary #25212

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

Closed

Conversation

amenk
Copy link
Contributor

@amenk amenk commented Oct 22, 2019

see Ticket #25199

@amenk amenk requested a review from akaplya as a code owner October 22, 2019 10:56
@m2-assistant
Copy link

m2-assistant bot commented Oct 22, 2019

Hi @amenk. Thank you for your contribution
Here is some useful tips how you can test your changes using Magento test environment.
Add the comment under your pull request to deploy test or vanilla Magento instance:

  • @magento give me test instance - deploy test instance based on PR changes
  • @magento give me 2.3-develop instance - deploy vanilla Magento instance

For more details, please, review the Magento Contributor Guide documentation.

@amenk
Copy link
Contributor Author

amenk commented Nov 22, 2019

Actually the approach to threat all possible integers in all columns might cause problems for varchars.

Because of this behavior in MySQL: http://sqlfiddle.com/#!9/1c01c9/1/0

While my patch is fine for entity_id fields, now if Magento tries to match an SKU like (string) '1', my patch would convert this to a (int) 1 and match also SKUs like '1-2', '1-3' and so on.

@amenk
Copy link
Contributor Author

amenk commented Nov 22, 2019

Alternative patch:

vendor/magento/module-catalog/Model/Indexer/Category/Product/Action/Full.php:285

    foreach ($batchQueries as $query) {
        $this->connection->delete($this->tableMaintainer->getMainTmpTable((int)$store->getId()));
        $entityIds = $this->connection->fetchCol($query);
        foreach($entityIds as &$val) {  ##patch
            $val = (integer) $val; ## patch
        } ##patch
        $resultSelect = clone $basicSelect;
        $resultSelect->where($whereCondition, $entityIds);
        $this->connect

@mropanen
Copy link

mropanen commented Dec 3, 2019

Also strings like '23523523e2353253562356' are is_numeric == true ;)
I ended up doing this as a quick fix:
if (is_numeric($val) && $val < INF) {

@amenk
Copy link
Contributor Author

amenk commented Dec 3, 2019

@mropanen you have to be careful to not threat numeric SKUs as int. This can have the side effects I described.

@sidolov sidolov changed the base branch from 2.3-develop to 2.4-develop December 5, 2019 17:17
@robert-scheck
Copy link

While the patch indeed solves the issue with "Category Products index", it doesn't address the (same?) issue at "Product Price index". Just ending up here with:

SQLSTATE[HY000]: General error: 1205 Lock wait timeout exceeded; try restarting transaction, query was: DELETE FROM `catalog_product_index_website`

Aside of that, it feels like the patch makes the indexer slower (factor ~ 1.5) in general.

@ihor-sviziev ihor-sviziev self-assigned this May 7, 2020
@ihor-sviziev ihor-sviziev added the Severity: S1 Affects critical data or functionality and forces users to employ a workaround. label May 7, 2020
@ihor-sviziev
Copy link
Contributor

Hi @amenk ,
I see that we have two solutions for fixing this issue. Just reviewed both of them and I see that following solution is better https://github.com/magento/magento2/pull/27129/files#diff-ffe9c9bcf0327071a617f8ba7bbfde56R285 as it's fixes root of the issue and not affecting almost all queries in the magento and custom extensions.

As we have better solution - I'm closing this PR.
Thank you so much for contribution!

@m2-assistant
Copy link

m2-assistant bot commented May 8, 2020

Hi @amenk, thank you for your contribution!
Please, complete Contribution Survey, it will take less than a minute.
Your feedback will help us to improve contribution process.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Component: DB Progress: pending review Release Line: 2.4 Severity: S1 Affects critical data or functionality and forces users to employ a workaround. Squashtoberfest 2019
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Catalog Category Indexing takes very long on MariaDB 10.3 with many products
7 participants