Merge pull request #4788 from magento-tango/MC-19791

[tango] MC-19791: Poor performance on sales order update - string to integer
magento · Sep 18, 2019 · 0a390bb · 0a390bb
2 parents d9d4b82 + 2c800de
commit 0a390bb
Show file tree

Hide file tree

Showing 3 changed files with 44 additions and 13 deletions.
diff --git a/app/code/Magento/Customer/Test/Unit/Model/ResourceModel/GroupTest.php b/app/code/Magento/Customer/Test/Unit/Model/ResourceModel/GroupTest.php
@@ -127,7 +127,7 @@ public function testSaveWithReservedId()
                 ]
             )
             ->getMockForAbstractClass();
-        $dbAdapter->expects($this->any())->method('describeTable')->willReturn([]);
+        $dbAdapter->expects($this->any())->method('describeTable')->willReturn(['customer_group_id' => []]);
         $dbAdapter->expects($this->any())->method('update')->willReturnSelf();
         $dbAdapter->expects($this->once())->method('lastInsertId')->willReturn($expectedId);
         $selectMock = $this->getMockBuilder(\Magento\Framework\DB\Select::class)

diff --git a/lib/internal/Magento/Framework/Model/ResourceModel/Db/AbstractDb.php b/lib/internal/Magento/Framework/Model/ResourceModel/Db/AbstractDb.php
@@ -784,6 +784,24 @@ protected function saveNewObject(\Magento\Framework\Model\AbstractModel $object)
         }
     }
 
+    /**
+     * Check if column data type is numeric
+     *
+     * Based on column description
+     *
+     * @param array $columnDescription
+     * @return bool
+     */
+    private function isNumericValue(array $columnDescription): bool
+    {
+        $result = true;
+        if (!empty($columnDescription['DATA_TYPE'])
+            && in_array($columnDescription['DATA_TYPE'], ['tinyint', 'smallint', 'mediumint', 'int', 'bigint'])) {
+            $result = false;
+        }
+        return $result;
+    }
+
     /**
      * Update existing object
      *
@@ -793,29 +811,35 @@ protected function saveNewObject(\Magento\Framework\Model\AbstractModel $object)
      */
     protected function updateObject(\Magento\Framework\Model\AbstractModel $object)
     {
-        $condition = $this->getConnection()->quoteInto($this->getIdFieldName() . '=?', $object->getId());
+        $connection = $this->getConnection();
+        $tableDescription = $connection->describeTable($this->getMainTable());
+        $preparedValue = $connection->prepareColumnValue($tableDescription[$this->getIdFieldName()], $object->getId());
+        $condition  = (!$this->isNumericValue($tableDescription[$this->getIdFieldName()]))
+            ? sprintf('%s=%d', $this->getIdFieldName(), $preparedValue)
+            : $connection->quoteInto($this->getIdFieldName() . '=?', $preparedValue);
+
         /**
          * Not auto increment primary key support
          */
         if ($this->_isPkAutoIncrement) {
             $data = $this->prepareDataForUpdate($object);
             if (!empty($data)) {
-                $this->getConnection()->update($this->getMainTable(), $data, $condition);
+                $connection->update($this->getMainTable(), $data, $condition);
             }
         } else {
-            $select = $this->getConnection()->select()->from(
+            $select = $connection->select()->from(
                 $this->getMainTable(),
                 [$this->getIdFieldName()]
             )->where(
                 $condition
             );
-            if ($this->getConnection()->fetchOne($select) !== false) {
+            if ($connection->fetchOne($select) !== false) {
                 $data = $this->prepareDataForUpdate($object);
                 if (!empty($data)) {
-                    $this->getConnection()->update($this->getMainTable(), $data, $condition);
+                    $connection->update($this->getMainTable(), $data, $condition);
                 }
             } else {
-                $this->getConnection()->insert(
+                $connection->insert(
                     $this->getMainTable(),
                     $this->_prepareDataForSave($object)
                 );

diff --git a/lib/internal/Magento/Framework/Model/Test/Unit/ResourceModel/Db/AbstractDbTest.php b/lib/internal/Magento/Framework/Model/Test/Unit/ResourceModel/Db/AbstractDbTest.php
@@ -425,16 +425,15 @@ public function testPrepareDataForUpdate()
         $connectionMock = $this->getMockBuilder(AdapterInterface::class)
             ->setMethods(['save'])
             ->getMockForAbstractClass();
+
         $context = (new \Magento\Framework\TestFramework\Unit\Helper\ObjectManager($this))->getObject(
             \Magento\Framework\Model\Context::class
         );
         $registryMock = $this->createMock(\Magento\Framework\Registry::class);
-        $resourceMock = $this->createPartialMock(AbstractDb::class, [
-                '_construct',
-                'getConnection',
-                '__wakeup',
-                'getIdFieldName'
-            ]);
+        $resourceMock = $this->createPartialMock(
+            AbstractDb::class,
+            ['_construct', 'getConnection', '__wakeup', 'getIdFieldName']
+        );
         $connectionInterfaceMock = $this->createMock(AdapterInterface::class);
         $resourceMock->expects($this->any())
             ->method('getConnection')
@@ -453,6 +452,7 @@ public function testPrepareDataForUpdate()
         $this->_resourcesMock->expects($this->any())->method('getTableName')->with($data)->will(
             $this->returnValue('tableName')
         );
+
         $mainTableReflection = new \ReflectionProperty(
             AbstractDb::class,
             '_mainTable'
@@ -467,6 +467,13 @@ public function testPrepareDataForUpdate()
         $idFieldNameReflection->setValue($this->_model, 'idFieldName');
         $connectionMock->expects($this->any())->method('save')->with('tableName', 'idFieldName');
         $connectionMock->expects($this->any())->method('quoteInto')->will($this->returnValue('idFieldName'));
+        $connectionMock->expects($this->any())
+            ->method('describeTable')
+            ->with('tableName')
+            ->willReturn(['idFieldName' => []]);
+        $connectionMock->expects($this->any())
+            ->method('prepareColumnValue')
+            ->willReturn(0);
         $abstractModelMock->setIdFieldName('id');
         $abstractModelMock->setData(
             [