Drop redundant index on cart_rule_combination - #1898
Open
ShaiMagal wants to merge 1 commit into
Open
Conversation
Companion migration for PrestaShop/PrestaShop#42438, which removes KEY id_cart_rule_1 from the canonical schema. The index is the leftmost prefix of PRIMARY KEY (id_cart_rule_1, id_cart_rule_2), so it can never be selected over the primary key and only costs disk space and write time. Existing shops keep it forever without this migration, since the schema is only ever moved forward by these scripts. The statement is standalone on purpose. A failing DROP INDEX raises error 1091, which the runner already lists as safely ignorable (MysqlErrorCode::CANNOT_DROP_KEY), so a shop that no longer has the index is not affected. Putting it inside a multi-clause ALTER would make that same error discard the other clauses.
|
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.



KEY id_cart_rule_1fromcart_rule_combinationin the canonical schema, because it is the leftmost prefix ofPRIMARY KEY (id_cart_rule_1, id_cart_rule_2)and therefore can never be selected over the primary key. Since the schema of an existing shop is only ever moved forward by the scripts inupgrade/sql/, the core PR alone would only ever benefit fresh installations. This adds the matching statement so upgraded shops get it too.SHOW INDEX FROM ps_cart_rule_combinationlistsPRIMARY,id_cart_rule_1andid_cart_rule_2. Run the update to 9.3.0 and check that onlyPRIMARYandid_cart_rule_2remain. Run it a second time (or on a shop that never had the index) and check that the update still finishes without a warning: the statement then fails with error 1091, which is already in the ignore list.Why it matters
cart_rule_combinationis one of the biggest tables on shops that generate one voucher per customer, because it grows quadratically with the number of cart rules. On a production shop I looked at (PrestaShop 8.1.7, about 14 000 cart rules) it holds 69 555 681 rows and takes 4 289 MB, of which 2 159 MB is secondary indexes.Both secondary indexes cover the same two
int unsignedcolumns and InnoDB appends the missing primary key column to each, so the two leaf structures are the same size. Dropping the redundant one frees about half of that index space, roughly 1 GB on that shop, and removes one index maintenance operation from every insert and delete on the table.Why the statement is standalone
DROP INDEXon a missing index raises error 1091, whichCoreUpgraderalready lists as safely ignorable viaMysqlErrorCode::CANNOT_DROP_KEY, so shops that no longer have the index are unaffected.I deliberately did not fold it into a multi-clause
ALTER TABLE: that is exactly the shape that causes #1636, where a failingDROP INDEXdiscards the other clauses of the same statement. If adrop_index_if_exists.phphelper is added later (as discussed in #1636), this line is a one-word change.File choice
I put it in a new
upgrade/sql/9.3.0.sqlbecause the core PR targetsdevelop, which is 9.3.0. If you would rather have it in9.2.0.sql, say the word and I will move it - though shops already running 9.2.0 would then never receive it, since the runner only applies scripts whose version is strictly greater than the shop version.