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

Add index on salesrule_coupon #2959

Closed
wants to merge 2 commits into from
Closed

Add index on salesrule_coupon #2959

wants to merge 2 commits into from

Conversation

luigifab
Copy link
Contributor

Description

We have 220,000 coupons in salesrule_coupon.
This PR add an index on code column, like #2447.

Here is a before/after with Blackfire:

coupons-before-after

Contribution checklist

  • Pull request has a meaningful description of its purpose
  • All commits are accompanied by meaningful commit messages
  • All automated tests passed successfully (all builds are green)
  • Add yourself to contributors list

@github-actions github-actions bot added the Component: SalesRule Relates to Mage_SalesRule label Jan 17, 2023
@luigifab luigifab added the performance Performance related label Jan 17, 2023
@tmotyl
Copy link
Contributor

tmotyl commented Jan 17, 2023

what query is being run?

@luigifab
Copy link
Contributor Author

Yes, I forgot to copy/paste: select salesrule_coupon.* from salesrule_coupon wehere (salesrule_coupon.code = ?)

@kiatng
Copy link
Contributor

kiatng commented Jan 18, 2023

The column code is

UNIQUE KEY `UNQ_SALESRULE_COUPON_CODE` (`code`),

According to this stackoveflow, it's unnecessary to index it again. I am not sure how to interpret Blackfire output, is stackoverflow wrong?

[edit]

The unique index serves a dual purpose. It functions just like any other index when you perform a query based on a phone number: ...However, it also checks every value when attempting to insert or update a record to ensure that the value doesn’t already exist. In this way, the unique index acts as a constraint.

Unique indexes use as much space as nonunique indexes do. The value of every column as well as the record’s location is stored. This can be a waste if you use the unique index as a constraint and never as an index. Put another way, you may rely on the unique index to enforce uniqueness but never write a query that uses the unique value. In this case, there’s no need for MySQL to store the locations of every record in the index: you’ll never use them.

Unfortunately, there’s no way to signal your intentions to MySQL. In the future, we’ll likely find a feature introduced for this specific case. The MyISAM storage engine already has support for unique columns without an index (it uses a hash-based system), but the mechanism isn’t exposed at the SQL level yet.

From https://www.oreilly.com/library/view/high-performance-mysql/0596003064/ch04.html

@luigifab
Copy link
Contributor Author

Damned, you are right. Now I remember that the store use a module that removed the unique key to allow multiple coupon with same code.

@fballiano
Copy link
Contributor

that's @kiatng for the finding!

I'm closing this one but I still think we should merge #2447

@fballiano fballiano closed this Jan 18, 2023
@luigifab luigifab deleted the index-coupon branch January 18, 2023 09:50
@sreichel
Copy link
Contributor

but I still think we should merge #2447

We already merged it.

@fballiano
Copy link
Contributor

I was meaning #1110 :-D

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Component: SalesRule Relates to Mage_SalesRule performance Performance related
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants