mysql 5.7 でdeadlockが発生 #630

- トランザクション分離レベルを意識して、無駄な SQL 実行を避ける。#630 (comment)
- 幾らかコストが低い exists() を使う。#630 (comment)
- SC_DB_DBFactory のテストが無さそうなので、今回追加したメソッドの他、ごく基本的な部分のみ実装する。

data/class/SC_Query.php

@@ -832,8 +832,12 @@ public function getCol($col, $table = '', $where = '', $arrWhereVal = [])
      */
     public function delete($table, $where = '', $arrWhereVal = [])
     {
-        // 空deleteは実行しない
-        if ($this->count($table, $where, $arrWhereVal) == 0) {
+        // デッドロックを生じ得る条件の場合、空 DELETE を避ける
+        if (
+            $this->inTransaction()
+            && $this->dbFactory->isSkipDeleteIfNotExists()
+            && !$this->exists($table, $where, $arrWhereVal)
+        ) {
             return;
         }
 

data/class/db/SC_DB_DBFactory.php

@@ -30,6 +30,11 @@
  */
 class SC_DB_DBFactory
 {
+    public const ISOLATION_LEVEL_READ_UNCOMMITTED = 10;
+    public const ISOLATION_LEVEL_READ_COMMITTED = 20;
+    public const ISOLATION_LEVEL_REPEATABLE_READ = 30;
+    public const ISOLATION_LEVEL_SERIALIZABLE = 40;
+
     /**
      * DB_TYPE に応じた DBFactory インスタンスを生成する.
      *
@@ -321,4 +326,26 @@ public function alldtlSQL($where_products_class = '', $product_ids = [])
 
         return $sql;
     }
+
+    /**
+     * トランザクション分離レベルを取得する。
+     *
+     * @return int
+     */
+    public function getTransactionIsolationLevel()
+    {
+        // TODO: 一般的な DBMS のデフォルトを返している。実際のレベルを返すのが望ましい。しかし、毎回 `SHOW transaction_isolation` などを実行するのは避けたい。インストーラーで実行環境のレベルを退避して設定ファイルに記録したり、設定ファイルで別のレベルを設定できるよう改善できそう。
+        return static::ISOLATION_LEVEL_READ_COMMITTED;
+    }
+
+    /**
+     * 削除時に対象行が存在しない場合に削除をスキップする必要があるかを返す。
+     *
+     * @return bool
+     */
+
+    public function isSkipDeleteIfNotExists()
+    {
+        return false;
+    }
 }

data/class/db/dbfactory/SC_DB_DBFactory_MYSQL.php

@@ -389,4 +389,15 @@ public function initObjQuery(SC_Query &$objQuery)
         }
         $objQuery->exec("SET SESSION sql_mode = 'ANSI'");
     }
-}
+
+    public function getTransactionIsolationLevel()
+    {
+        // TODO: デフォルトを返している。実際のレベルを返すのが望ましい。しかし、毎回 `SELECT @@session.transaction_isolation` などを実行するのは避けたい。
+        return static::ISOLATION_LEVEL_REPEATABLE_READ;
+    }
+
+     public function isSkipDeleteIfNotExists()
+     {
+         return $this->getTransactionIsolationLevel() >= static::ISOLATION_LEVEL_REPEATABLE_READ;
+     }
+ }

phpunit.xml.dist

@@ -31,7 +31,6 @@
         <directory>./data/class/pages/</directory>
         <directory>./data/class/graph/</directory>
         <directory>./data/class/batch/</directory>
-        <directory>./data/class/db/dbfactory/</directory>
       </exclude>
     </coverage>
 </phpunit>

tests/class/db/SC_DB_DBFactoryTest.php

@@ -0,0 +1,48 @@
+<?php
+
+class SC_DB_DBFactoryTest extends Common_TestCase
+{
+    /**
+     * @var SC_DB_DBFactory_PGSQL
+     */
+    protected $dbFactoryPgsql;
+
+    /**
+     * @var SC_DB_DBFactory_MYSQL
+     */
+    protected $dbFactoryMysql;
+
+    /**
+     * @var SC_DB_DBFactory_MYSQL
+     */
+    protected $dbFactoryMysqli;
+
+    protected function setUp(): void
+    {
+        parent::setUp();
+        $this->dbFactoryPgsql = SC_DB_DBFactory_Ex::getInstance('pgsql');
+        $this->dbFactoryMysql = SC_DB_DBFactory_Ex::getInstance('mysql');
+        $this->dbFactoryMysqli = SC_DB_DBFactory_Ex::getInstance('mysqli');
+    }
+
+    public function testGetInstance_pgsql()
+    {
+        $this->assertInstanceOf('SC_DB_DBFactory', $this->dbFactoryPgsql);
+        $this->assertInstanceOf('SC_DB_DBFactory_Ex', $this->dbFactoryPgsql);
+        $this->assertInstanceOf('SC_DB_DBFactory_PGSQL', $this->dbFactoryPgsql);
+        $this->assertInstanceOf('SC_DB_DBFactory_PGSQL_Ex', $this->dbFactoryPgsql);
+    }
+
+    public function testGetInstance_mysql()
+    {
+        $this->assertInstanceOf('SC_DB_DBFactory', $this->dbFactoryMysql);
+        $this->assertInstanceOf('SC_DB_DBFactory_Ex', $this->dbFactoryMysql);
+        $this->assertInstanceOf('SC_DB_DBFactory_MYSQL', $this->dbFactoryMysql);
+        $this->assertInstanceOf('SC_DB_DBFactory_MYSQL_Ex', $this->dbFactoryMysql);
+    }
+
+    public function testGetInstance_mysqli()
+    {
+        $this->assertSame(get_class($this->dbFactoryMysql), get_class($this->dbFactoryMysqli));
+    }
+}

tests/class/db/dbfactory/SC_DB_DBFactory_MYSQLTest.php

@@ -0,0 +1,30 @@
+<?php
+
+class SC_DB_DBFactory_MYSQLTest extends Common_TestCase
+{
+    /**
+     * @var SC_DB_DBFactory_MYSQL
+     */
+    protected $dbFactory;
+
+    protected function setUp(): void
+    {
+        parent::setUp();
+        $this->dbFactory = new SC_DB_DBFactory_MYSQL_Ex;
+    }
+
+    public function testGetInstance()
+    {
+        $this->assertInstanceOf('SC_DB_DBFactory_MYSQL_Ex', $this->dbFactory);
+    }
+
+    public function testGetTransactionIsolationLevel()
+    {
+        $this->assertEquals(SC_DB_DBFactory_Ex::ISOLATION_LEVEL_REPEATABLE_READ, $this->dbFactory->getTransactionIsolationLevel());
+    }
+
+    public function testIsSkipDeleteIfNotExists()
+    {
+        $this->assertEquals(true, $this->dbFactory->isSkipDeleteIfNotExists());
+    }
+}

tests/class/db/dbfactory/SC_DB_DBFactory_PGSQLTest.php

@@ -0,0 +1,30 @@
+<?php
+
+class SC_DB_DBFactory_PGSQLTest extends Common_TestCase
+{
+    /**
+     * @var SC_DB_DBFactory_PGSQL
+     */
+    protected $dbFactory;
+
+    protected function setUp(): void
+    {
+        parent::setUp();
+        $this->dbFactory = new SC_DB_DBFactory_PGSQL_Ex;
+    }
+
+    public function testGetInstance()
+    {
+        $this->assertInstanceOf('SC_DB_DBFactory_PGSQL_Ex', $this->dbFactory);
+    }
+
+    public function testGetTransactionIsolationLevel()
+    {
+        $this->assertEquals(SC_DB_DBFactory_Ex::ISOLATION_LEVEL_READ_COMMITTED, $this->dbFactory->getTransactionIsolationLevel());
+    }
+
+    public function testIsSkipDeleteIfNotExists()
+    {
+        $this->assertEquals(false, $this->dbFactory->isSkipDeleteIfNotExists());
+    }
+}