19117: fix performance leak in salesrule collection

david-fuehr · david-fuehr · commit fa7fb549db65 · 2019-01-22T11:43:24.000+01:00
Github Issue: #19117 Refactored sql query that created a huge temporary table for each request, when a greater amount of salesrules and coupon codes exists in database. The sorting of this table took a lot of cpu time. The statement now consists of two subselects that drill down the remaining lines as far as possible, so that the remaining temporary table is minimal and easily sorted. example: for 2,000 salesrules and 3,000,000 coupon codes the original query took about 2.4 seconds (mbp, server, aws). the optimized query takes about 5ms (about 100ms on aws).
diff --git a/app/code/Magento/SalesRule/Model/ResourceModel/Rule/Collection.php b/app/code/Magento/SalesRule/Model/ResourceModel/Rule/Collection.php
@@ -9,6 +9,9 @@
 use Magento\Framework\DB\Select;
 use Magento\Framework\Serialize\Serializer\Json;
 use Magento\Quote\Model\Quote\Address;
+use Magento\SalesRule\Api\Data\CouponInterface;
+use Magento\SalesRule\Model\Coupon;
+use Magento\SalesRule\Model\Rule;
 
 /**
  * Sales Rules resource collection model.
@@ -105,12 +108,15 @@ protected function mapAssociatedEntities($entityType, $objectField)
 
         $associatedEntities = $this->getConnection()->fetchAll($select);
 
-        array_map(function ($associatedEntity) use ($entityInfo, $ruleIdField, $objectField) {
-            $item = $this->getItemByColumnValue($ruleIdField, $associatedEntity[$ruleIdField]);
-            $itemAssociatedValue = $item->getData($objectField) === null ? [] : $item->getData($objectField);
-            $itemAssociatedValue[] = $associatedEntity[$entityInfo['entity_id_field']];
-            $item->setData($objectField, $itemAssociatedValue);
-        }, $associatedEntities);
+        array_map(
+            function ($associatedEntity) use ($entityInfo, $ruleIdField, $objectField) {
+                $item = $this->getItemByColumnValue($ruleIdField, $associatedEntity[$ruleIdField]);
+                $itemAssociatedValue = $item->getData($objectField) ?? [];
+                $itemAssociatedValue[] = $associatedEntity[$entityInfo['entity_id_field']];
+                $item->setData($objectField, $itemAssociatedValue);
+            },
+            $associatedEntities
+        );
     }
 
     /**
@@ -137,6 +143,7 @@ protected function _afterLoad()
      * @param string $couponCode
      * @param string|null $now
      * @param Address $address allow extensions to further filter out rules based on quote address
+     * @throws \Zend_Db_Select_Exception
      * @use $this->addWebsiteGroupDateFilter()
      * @SuppressWarnings(PHPMD.UnusedFormalParameter)
      * @return $this
@@ -149,84 +156,122 @@ public function setValidationFilter(
         Address $address = null
     ) {
         if (!$this->getFlag('validation_filter')) {
-            /* We need to overwrite joinLeft if coupon is applied */
-            $this->getSelect()->reset();
-            parent::_initSelect();
 
-            $this->addWebsiteGroupDateFilter($websiteId, $customerGroupId, $now);
-            $select = $this->getSelect();
+            $this->prepareSelect($websiteId, $customerGroupId, $now);
 
-            $connection = $this->getConnection();
-            if (strlen($couponCode)) {
-                $select->joinLeft(
-                    ['rule_coupons' => $this->getTable('salesrule_coupon')],
-                    $connection->quoteInto(
-                        'main_table.rule_id = rule_coupons.rule_id AND main_table.coupon_type != ?',
-                        \Magento\SalesRule\Model\Rule::COUPON_TYPE_NO_COUPON
-                    ),
-                    ['code']
-                );
-
-                $noCouponWhereCondition = $connection->quoteInto(
-                    'main_table.coupon_type = ? ',
-                    \Magento\SalesRule\Model\Rule::COUPON_TYPE_NO_COUPON
-                );
-
-                $autoGeneratedCouponCondition = [
-                    $connection->quoteInto(
-                        "main_table.coupon_type = ?",
-                        \Magento\SalesRule\Model\Rule::COUPON_TYPE_AUTO
-                    ),
-                    $connection->quoteInto(
-                        "rule_coupons.type = ?",
-                        \Magento\SalesRule\Api\Data\CouponInterface::TYPE_GENERATED
-                    ),
-                ];
-
-                $orWhereConditions = [
-                    "(" . implode($autoGeneratedCouponCondition, " AND ") . ")",
-                    $connection->quoteInto(
-                        '(main_table.coupon_type = ? AND main_table.use_auto_generation = 1 AND rule_coupons.type = 1)',
-                        \Magento\SalesRule\Model\Rule::COUPON_TYPE_SPECIFIC
-                    ),
-                    $connection->quoteInto(
-                        '(main_table.coupon_type = ? AND main_table.use_auto_generation = 0 AND rule_coupons.type = 0)',
-                        \Magento\SalesRule\Model\Rule::COUPON_TYPE_SPECIFIC
-                    ),
-                ];
-
-                $andWhereConditions = [
-                    $connection->quoteInto(
-                        'rule_coupons.code = ?',
-                        $couponCode
-                    ),
-                    $connection->quoteInto(
-                        '(rule_coupons.expiration_date IS NULL OR rule_coupons.expiration_date >= ?)',
-                        $this->_date->date()->format('Y-m-d')
-                    ),
-                ];
-
-                $orWhereCondition = implode(' OR ', $orWhereConditions);
-                $andWhereCondition = implode(' AND ', $andWhereConditions);
-
-                $select->where(
-                    $noCouponWhereCondition . ' OR ((' . $orWhereCondition . ') AND ' . $andWhereCondition . ')',
-                    null,
-                    Select::TYPE_CONDITION
-                );
+            $noCouponRules = $this->getNoCouponCodeSelect();
+
+            if ($couponCode) {
+                $couponRules = $this->getCouponCodeSelect($couponCode);
+
+                $allAllowedRules = $this->getConnection()->select();
+                $allAllowedRules->union([$noCouponRules, $couponRules], \Zend_Db_Select::SQL_UNION_ALL);
+
+                $this->_select = $allAllowedRules;
             } else {
-                $this->addFieldToFilter(
-                    'main_table.coupon_type',
-                    \Magento\SalesRule\Model\Rule::COUPON_TYPE_NO_COUPON
-                );
+                $this->_select = $noCouponRules;
             }
+
             $this->setOrder('sort_order', self::SORT_ORDER_ASC);
             $this->setFlag('validation_filter', true);
         }
 
         return $this;
     }
 
+    /**
+     * Recreate the default select object for specific needs of salesrule evaluation with coupon codes.
+     *
+     * @param $websiteId
+     * @param $customerGroupId
+     * @param $now
+     */
+    private function prepareSelect($websiteId, $customerGroupId, $now)
+    {
+        $this->getSelect()->reset();
+        parent::_initSelect();
+
+        $this->addWebsiteGroupDateFilter($websiteId, $customerGroupId, $now);
+    }
+
+    /**
+     * Return select object to determine all active rules not needing a coupon code.
+     *
+     * @return Select
+     */
+    private function getNoCouponCodeSelect()
+    {
+        $noCouponSelect = clone $this->getSelect();
+
+        $noCouponSelect->where(
+            'main_table.coupon_type = ?',
+            Rule::COUPON_TYPE_NO_COUPON
+        );
+
+        $noCouponSelect->columns([Coupon::KEY_CODE => new \Zend_Db_Expr('NULL')]);
+
+        return $noCouponSelect;
+    }
+
+    /**
+     * Determine all active rules that are valid for the given coupon code.
+     *
+     * @param $couponCode
+     * @return Select
+     */
+    private function getCouponCodeSelect($couponCode)
+    {
+        $couponSelect = clone $this->getSelect();
+
+        $this->joinCouponTable($couponCode, $couponSelect);
+
+        $notExpired = $this->getConnection()->quoteInto(
+            '(rule_coupons.expiration_date IS NULL OR rule_coupons.expiration_date >= ?)',
+            $this->_date->date()->format('Y-m-d')
+        );
+
+        $isAutogeneratedCoupon =
+            $this->getConnection()->quoteInto('main_table.coupon_type = ?', Rule::COUPON_TYPE_AUTO)
+            . ' AND ' .
+            $this->getConnection()->quoteInto('rule_coupons.type = ?', CouponInterface::TYPE_GENERATED);
+
+        $isValidSpecificCoupon =
+            $this->getConnection()->quoteInto('(main_table.coupon_type = ?)', Rule::COUPON_TYPE_SPECIFIC)
+            . ' AND (' .
+            '(main_table.use_auto_generation = 1 AND rule_coupons.type = 1)'
+            . ' OR ' .
+            '(main_table.use_auto_generation = 0 AND rule_coupons.type = 0)'
+            . ')';
+
+        $couponSelect->where(
+            "$notExpired AND ($isAutogeneratedCoupon OR $isValidSpecificCoupon)",
+            null,
+            Select::TYPE_CONDITION
+        );
+
+        return $couponSelect;
+    }
+
+    /**
+     * @param $couponCode
+     * @param Select $couponSelect
+     */
+    private function joinCouponTable($couponCode, Select $couponSelect)
+    {
+        $couponJoinCondition =
+            'main_table.rule_id = rule_coupons.rule_id'
+            . ' AND ' .
+            $this->getConnection()->quoteInto('main_table.coupon_type <> ?', Rule::COUPON_TYPE_NO_COUPON)
+            . ' AND ' .
+            $this->getConnection()->quoteInto('rule_coupons.code = ?', $couponCode);
+
+        $couponSelect->joinInner(
+            ['rule_coupons' => $this->getTable('salesrule_coupon')],
+            $couponJoinCondition,
+            [Coupon::KEY_CODE]
+        );
+    }
+
     /**
      * Filter collection by website(s), customer group(s) and date.
      * Filter collection to only active rules.
