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

Improving query performance for coupon code check #2973

Closed
wants to merge 3 commits into from
Closed

Improving query performance for coupon code check #2973

wants to merge 3 commits into from

Conversation

luigifab
Copy link
Contributor

@luigifab luigifab commented Jan 19, 2023

Description

After #1110 (or before for me), I found a problem in cart.
Before this PR and before #1110, with ~220,000 coupons, in checkout/cart:

image

before-global-sql

With #1110:

image

After both PR:

image

See comment.

My solution is to create a sub query.
For us this is working, or we haven't yet found the new problem / side effect.

SQL query before

SELECT `main_table`.*, `rule_coupons`.`code`
FROM `salesrule` AS `main_table`
INNER JOIN `salesrule_customer_group` AS `customer_group_ids`
  ON main_table.rule_id = customer_group_ids.rule_id
  AND customer_group_ids.customer_group_id = 0
LEFT JOIN `salesrule_coupon` AS `rule_coupons`
  ON main_table.rule_id = rule_coupons.rule_id
  AND main_table.coupon_type != 1
  AND rule_coupons.code = 'abc' # PR 1110
WHERE (EXISTS
         (SELECT 1
          FROM `salesrule_website` AS `website`
          WHERE (website.website_id IN ('1'))
            AND (main_table.rule_id = website.rule_id)))
  AND (from_date IS NULL OR from_date <= '2023-01-19')
  AND (to_date IS NULL OR to_date >= '2023-01-19')
  AND (`is_active` = '1')
  AND (
    main_table.coupon_type = 1
    OR (
      (
        (main_table.coupon_type = 3 AND rule_coupons.type = 0)
        OR (main_table.coupon_type = 2 AND main_table.use_auto_generation = 1 AND rule_coupons.type = 1)
        OR (main_table.coupon_type = 2 AND main_table.use_auto_generation = 0 AND rule_coupons.type = 0)
      )
      AND rule_coupons.code = 'abc'
    )
  );

SQL query after

SELECT `main_table`.*, `rule_coupons`.`code`
FROM `salesrule` AS `main_table`
INNER JOIN `salesrule_customer_group` AS `customer_group_ids`
  ON main_table.rule_id = customer_group_ids.rule_id
  AND customer_group_ids.customer_group_id = 0
LEFT JOIN `salesrule_coupon` AS `rule_coupons`
  ON main_table.rule_id = rule_coupons.rule_id
  AND main_table.coupon_type != 1
  AND rule_coupons.code = 'abc' # PR 1110
WHERE (EXISTS
         (SELECT 1
          FROM `salesrule_website` AS `website`
          WHERE (website.website_id IN ('1'))
            AND (main_table.rule_id = website.rule_id)))
  AND (from_date IS NULL OR from_date <= '2023-01-19')
  AND (to_date IS NULL OR to_date >= '2023-01-19')
  AND (`is_active` = '1')
  AND (
    main_table.coupon_type = 1
    OR main_table.rule_id IN
       (SELECT DISTINCT `main_table`.`rule_id`
        FROM `salesrule` AS `main_table`
        INNER JOIN `salesrule_customer_group` AS `customer_group_ids`
          ON main_table.rule_id = customer_group_ids.rule_id
          AND customer_group_ids.customer_group_id = 0
        LEFT JOIN `salesrule_coupon` AS `rule_coupons`
          ON main_table.rule_id = rule_coupons.rule_id
          AND main_table.coupon_type != 1
          AND rule_coupons.code = 'abc' # PR 1110
        WHERE (EXISTS
                 (SELECT 1
                  FROM `salesrule_website` AS `website`
                  WHERE (website.website_id IN ('1'))
                    AND (main_table.rule_id = website.rule_id)))
          AND (from_date IS NULL OR from_date <= '2023-01-19')
          AND (to_date IS NULL OR to_date >= '2023-01-19')
          AND (`is_active` = '1')
          AND (
            (
              (main_table.coupon_type = 3 AND rule_coupons.type = 0)
              OR (main_table.coupon_type = 2 AND main_table.use_auto_generation = 1 AND rule_coupons.type = 1)
              OR (main_table.coupon_type = 2 AND main_table.use_auto_generation = 0 AND rule_coupons.type = 0)
            )
            AND rule_coupons.code = 'abc'
          )
        ORDER BY `sort_order` ASC)
   )
GROUP BY `main_table`.`rule_id`;

Like @kiatng says, we can replace:

OR (main_table.coupon_type = 2 AND main_table.use_auto_generation = 1 AND rule_coupons.type = 1)
OR (main_table.coupon_type = 2 AND main_table.use_auto_generation = 0 AND rule_coupons.type = 0)

with:

OR (main_table.coupon_type = 2 AND main_table.use_auto_generation <= 1 AND rule_coupons.type <= 1)

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 19, 2023
@luigifab luigifab added the performance Performance related label Jan 19, 2023
matteotestoni
matteotestoni previously approved these changes Apr 5, 2023
@luigifab luigifab changed the base branch from 1.9.4.x to main April 9, 2023 08:05
@luigifab luigifab dismissed matteotestoni’s stale review April 9, 2023 08:05

The base branch was changed.

@luigifab luigifab changed the base branch from main to 1.9.4.x April 9, 2023 08:05
@luigifab luigifab changed the base branch from 1.9.4.x to main April 9, 2023 08:06
@kyrena
Copy link
Contributor

kyrena commented Jun 23, 2023

With #2102, maybe this PR is good, but it can be better.
Not sure about real impact.

But with this PR ANALYZE say:
image

With the SQL of the discussion, ANALYZE say:
image

@fballiano
Copy link
Contributor

I did some tests (creating 500.000 coupons), I don't know if I did something wrong but both this PR and #2102 where slower than the code that's already in the core.

I measured the time of the select with

                $time = microtime();
                $connection->fetchAll($select);
                echo microtime() - $time;

code from this PR (microtime difference):
Screenshot 2023-09-19 alle 00 09 21

code from without the PR:
Screenshot 2023-09-19 alle 00 10 35

The difference is negligible but...

Am I making any mistake?

@luigifab
Copy link
Contributor Author

luigifab commented Oct 5, 2023

Strange, sadly our Blackfire instance is dead for now, I can't test again now.

@luigifab luigifab marked this pull request as draft October 5, 2023 17:41
@luigifab
Copy link
Contributor Author

Close as can't test it again.

@luigifab luigifab closed this Nov 30, 2023
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.

4 participants