Skip to content

Commit

Permalink
19117: fix performance leak in salesrule collection
Browse files Browse the repository at this point in the history 
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).
  • Loading branch information
@david-fuehr
david-fuehr committed Mar 11, 2019
1 parent a089cfe commit 9a839d1
Showing 1 changed file with 101 additions and 84 deletions.
185 changes: 101 additions & 84 deletions app/code/Magento/SalesRule/Model/ResourceModel/Rule/Collection.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -107,12 +107,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) === null ? [] : $item->getData($objectField);
$itemAssociatedValue[] = $associatedEntity[$entityInfo['entity_id_field']];
$item->setData($objectField, $itemAssociatedValue);
},
$associatedEntities
);
}

/**
Expand Down Expand Up @@ -144,6 +147,7 @@ protected function _afterLoad()
* @use $this->addWebsiteGroupDateFilter()
* @SuppressWarnings(PHPMD.UnusedFormalParameter)
* @return $this
* @throws \Zend_Db_Select_Exception
*/
public function setValidationFilter(
$websiteId,
Expand All @@ -153,32 +157,21 @@ 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)) {
$noCouponWhereCondition = $connection->quoteInto(
'main_table.coupon_type = ?',
\Magento\SalesRule\Model\Rule::COUPON_TYPE_NO_COUPON
);
$relatedRulesIds = $this->getCouponRelatedRuleIds($couponCode);

$select->where(
$noCouponWhereCondition . ' OR main_table.rule_id IN (?)',
$relatedRulesIds,
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);
}
Expand All @@ -187,72 +180,96 @@ public function setValidationFilter(
}

/**
* Get rules ids related to coupon code
* Recreate the default select object for specific needs of salesrule evaluation with coupon codes.
*
* @param string $couponCode
* @return array
* @param $websiteId
* @param $customerGroupId
* @param $now
*/
private function getCouponRelatedRuleIds(string $couponCode): array
private function prepareSelect($websiteId, $customerGroupId, $now)
{
$connection = $this->getConnection();
$select = $connection->select()->from(
['main_table' => $this->getTable('salesrule')],
'rule_id'
$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
);
$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,
null
)

$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')
);

$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(
'(' . $orWhereCondition . ') AND ' . $andWhereCondition,
$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
);
$select->group('main_table.rule_id');

return $connection->fetchCol($select);
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]
);
}

/**
Expand Down

0 comments on commit 9a839d1

Please sign in to comment.