-
-
Notifications
You must be signed in to change notification settings - Fork 436
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
optimize url_rewrite performance #1240
Conversation
@greglamy do you have any stats showing the performance improvement? |
Just curious, does it still work without anything after |
Thanks for the PR! Please provide some queries with timing comparisons. The Also note that you could use an |
app/code/core/Mage/Catalog/sql/catalog_setup/upgrade-1.6.0.0.19.1.6-1.6.0.0.19.1.7.php
Outdated
Show resolved
Hide resolved
@greglamy you still interested in this PR? Looks like it would be useful. |
This modification is included in all my projects. |
/** @var Mage_Catalog_Model_Resource_Setup $installer */ | ||
$installer = $this; | ||
$connection = $installer->getConnection(); | ||
$connection->addColumn($installer->getTable('core_url_rewrite'), 'url_type', 'varchar(50) NULL AFTER `product_id`'); |
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.
I think it's missing an index here. Probably several indexes.
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.
or better like colinmollenhour say's: change varchar
to enum
?
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.
Either way, it should still have an index.
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.
If you leave off the AFTER product_id
part MySQL 8.0 will be able to add the column without copying the entire table which can reduce downtime while upgrading.
This looks to be a good change. Would love if someone who tried this with a large catalog can share their query times. |
Here is a script that you might find handy for this type of need to capture queries from a request. With it you can enable MySQL "general log" in seconds, watch the queries happening in real time, press Ctrl+C to stop monitoring and capture the output to a temporary file for later inspection. |
app/code/core/Mage/Catalog/sql/catalog_setup/upgrade-1.6.0.0.19.1.6-1.6.0.0.19.1.7.php
Outdated
Show resolved
Hide resolved
@joshua-bn @luigifab @colinmollenhour could you check your comments and decide what to do with this PR? |
I vote close if there is no activity for now as it is not proven if it has a benefit and there are some changes requested. A query on indexed varchar using a prefix match should be just about as fast or as fast as an indexed exact string comparison. It is a good observation, though. A proper compound index might give a better result. For example, this compound index:
was probably meant to address these queries but doesn't work - it should be changed to:
So that it can be used with |
it seems a very good idea so it deserves a "don't forget this pr" |
Description (*)
The goal of this PR is to optimize url rewrite performance for big catalogs.
Use of the 'like' operator in sql queries can generate particularly long execution times when the number of records concerned by the query is large.
Mage_Catalog_Model_Url defines a new field (url_type) for categories and products $rewriteData array.
SQL queries in Mage_Catalog_Helper_Category_Url_Rewrite and Mage_Catalog_Helper_Product_Url_Rewrite have been modified to avoid use of 'like' operator.
'eq' operator is used instead.
catalog_setup has been modified to :
Catalog version now defined as 1.6.0.0.19.1.7
Contribution checklist (*)