Drop the unused id_country index on address when upgrading - #1888
Open
boo-code wants to merge 1 commit into
Open
Drop the unused id_country index on address when upgrading#1888boo-code wants to merge 1 commit into
boo-code wants to merge 1 commit into
Conversation
Companion to PrestaShop/PrestaShop#42162, which removes the index from the install schema. Existing shops keep it until it is dropped for them, and it is the one that misleads the optimizer on the orders grid. Guarded through a PHP callback rather than a bare ALTER: shops that already dropped it by hand would otherwise fail the upgrade, and MySQL has no IF EXISTS for indexes.
Collaborator
|
Hello @boo-code! This is your first pull request on autoupgrade repository of the PrestaShop project. Thank you, and welcome to this Open Source community! |
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.



Companion to PrestaShop/PrestaShop#42162, which removes the
id_countryindex onaddressfrom the install schema. That side only changes new shops - existing ones keep the index, and they are the ones with enough orders to feel it.Why the index goes
It has no reader. Every core reference to
address.id_countryis a join againstcountrythat resolves throughcountry.PRIMARY-Customer.phptwice,CustomerAddressQueryBuilder,AddressQueryBuilder, andOrderCountriesChoiceProviderwhere the equality is againstcountry_lang. No core query filtersaddressby a literal country, and there is no foreign key on it.What it does do is mislead the optimizer on the orders grid. Measured on a development shop with 60 005 orders and 45 006 addresses, running the join shape that grid builds:
id_countrycountry_lang(ALL), thencountry, thenaddressviaid_country,Using temporary; Using filesortordersonPRIMARY,rows=50, backward index scan, no filesortPrestaShop/PrestaShop#41291 reports 4.7 s against 1.3 ms on 540k orders, which is the same plan flip at a larger scale.
Why a PHP callback and not a plain ALTER
9.0.1.sqldrops an index directly, so the bare statement would match precedent. It is the wrong shape here: the workaround circulating for #41291 is to runby hand, so on exactly the shops that were hurting the index is already gone, and
DROP INDEXon a missing index is an error. MySQL has noIF EXISTSfor indexes andDROP INDEX IF EXISTSis MariaDB only, so the check has to happen before the statement. The callback returnstruewhen there is nothing to drop.Version file
9.1.5.sql, matching the branch #42162 targets. If that PR is retargeted, this should move with it - say so and I will rename the file.What I could not run
The module's
.php-cs-fixer.dist.phploads a ruleset from the module's ownvendor/, which I do not have installed, so I did not run it and am not claiming a pass. The file follows the shape of the existing callbacks - same header, sameDbWrapperimport, same signature style.php -lis clean.