Performance optimizations

compositephp · Dec 23, 2023 · ba50060 · ba50060
1 parent 1ae4801
commit ba50060
Show file tree

Hide file tree

Showing 14 changed files with 204 additions and 63 deletions.
diff --git a/src/AbstractCachedTable.php b/src/AbstractCachedTable.php
@@ -8,7 +8,7 @@
 
 abstract class AbstractCachedTable extends AbstractTable
 {
-    use SelectRawTrait;
+    use Helpers\SelectRawTrait;
 
     protected const CACHE_VERSION = 1;
 
@@ -26,7 +26,7 @@ abstract protected function getFlushCacheKeys(AbstractEntity $entity): array;
     /**
      * @throws \Throwable
      */
-    public function save(AbstractEntity &$entity): void
+    public function save(AbstractEntity $entity): void
     {
         $cacheKeys = $this->collectCacheKeysByEntity($entity);
         parent::save($entity);
@@ -54,7 +54,7 @@ public function saveMany(array $entities): void
     /**
      * @throws \Throwable
      */
-    public function delete(AbstractEntity &$entity): void
+    public function delete(AbstractEntity $entity): void
     {
         $cacheKeys = $this->collectCacheKeysByEntity($entity);
         parent::delete($entity);

diff --git a/src/AbstractTable.php b/src/AbstractTable.php
@@ -2,21 +2,22 @@
 
 namespace Composite\DB;
 
+use Composite\DB\Exceptions\DbException;
 use Composite\DB\MultiQuery\MultiInsert;
 use Composite\DB\MultiQuery\MultiSelect;
-use Composite\Entity\Helpers\DateTimeHelper;
 use Composite\Entity\AbstractEntity;
-use Composite\DB\Exceptions\DbException;
+use Composite\Entity\Helpers\DateTimeHelper;
 use Doctrine\DBAL\Connection;
-use Doctrine\DBAL\Platforms\PostgreSQLPlatform;
 use Ramsey\Uuid\UuidInterface;
 
 abstract class AbstractTable
 {
-    use SelectRawTrait;
+    use Helpers\SelectRawTrait;
+    use Helpers\DatabaseSpecificTrait;
 
     protected readonly TableConfig $config;
 
+
     abstract protected function getConfig(): TableConfig;
 
     public function __construct()
@@ -44,49 +45,51 @@ public function getConnectionName(): string
      * @return void
      * @throws \Throwable
      */
-    public function save(AbstractEntity &$entity): void
+    public function save(AbstractEntity $entity): void
     {
         $this->config->checkEntity($entity);
         if ($entity->isNew()) {
             $connection = $this->getConnection();
             $this->checkUpdatedAt($entity);
 
-            $insertData = $this->formatData($entity->toArray());
+            $insertData = $this->prepareDataForSql($entity->toArray());
             $this->getConnection()->insert($this->getTableName(), $insertData);
 
-            if ($this->config->autoIncrementKey) {
-                $insertData[$this->config->autoIncrementKey] = intval($connection->lastInsertId());
-                $entity = $entity::fromArray($insertData);
-            } else {
-                $entity->resetChangedColumns();
+            if ($this->config->autoIncrementKey && ($lastInsertedId = $connection->lastInsertId())) {
+                $insertData[$this->config->autoIncrementKey] = intval($lastInsertedId);
+                $entity::schema()
+                    ->getColumn($this->config->autoIncrementKey)
+                    ->setValue($entity, $insertData[$this->config->autoIncrementKey]);
             }
+            $entity->resetChangedColumns($insertData);
         } else {
             if (!$changedColumns = $entity->getChangedColumns()) {
                 return;
             }
-            $connection = $this->getConnection();
-            $where = $this->getPkCondition($entity);
-
+            $changedColumns = $this->prepareDataForSql($changedColumns);
             if ($this->config->hasUpdatedAt() && property_exists($entity, 'updated_at')) {
                 $entity->updated_at = new \DateTimeImmutable();
                 $changedColumns['updated_at'] = DateTimeHelper::dateTimeToString($entity->updated_at);
             }
+            $whereParams = $this->getPkCondition($entity);
             if ($this->config->hasOptimisticLock()
                 && method_exists($entity, 'getVersion')
                 && method_exists($entity, 'incrementVersion')) {
-                $where['lock_version'] = $entity->getVersion();
+                $whereParams['lock_version'] = $entity->getVersion();
                 $entity->incrementVersion();
                 $changedColumns['lock_version'] = $entity->getVersion();
             }
-            $entityUpdated = $connection->update(
-                table: $this->getTableName(),
-                data: $changedColumns,
-                criteria: $where,
+            $updateString = implode(', ', array_map(fn ($key) => $this->escapeIdentifier($key) . "=?", array_keys($changedColumns)));
+            $whereString = implode(' AND ', array_map(fn ($key) => $this->escapeIdentifier($key) . "=?", array_keys($whereParams)));
+
+            $entityUpdated = (bool)$this->getConnection()->executeStatement(
+                sql: "UPDATE " . $this->escapeIdentifier($this->getTableName()) . " SET $updateString WHERE $whereString;",
+                params: array_merge(array_values($changedColumns), array_values($whereParams)),
             );
             if ($this->config->hasOptimisticLock() && !$entityUpdated) {
                 throw new Exceptions\LockException('Failed to update entity version, concurrency modification, rolling back.');
             }
-            $entity->resetChangedColumns();
+            $entity->resetChangedColumns($changedColumns);
         }
     }
 
@@ -101,7 +104,7 @@ public function saveMany(array $entities): void
             if ($entity->isNew()) {
                 $this->config->checkEntity($entity);
                 $this->checkUpdatedAt($entity);
-                $rowsToInsert[] = $this->formatData($entity->toArray());
+                $rowsToInsert[] = $this->prepareDataForSql($entity->toArray());
                 unset($entities[$i]);
             }
         }
@@ -113,14 +116,15 @@ public function saveMany(array $entities): void
             }
             if ($rowsToInsert) {
                 $chunks = array_chunk($rowsToInsert, 1000);
+                $connection = $this->getConnection();
                 foreach ($chunks as $chunk) {
                     $multiInsert = new MultiInsert(
+                        connection: $connection,
                         tableName: $this->getTableName(),
                         rows: $chunk,
                     );
                     if ($multiInsert->getSql()) {
-                        $stmt = $this->getConnection()->prepare($multiInsert->getSql());
-                        $stmt->executeQuery($multiInsert->getParameters());
+                        $connection->executeStatement($multiInsert->getSql(), $multiInsert->getParameters());
                     }
                 }
             }
@@ -135,7 +139,7 @@ public function saveMany(array $entities): void
      * @param AbstractEntity $entity
      * @throws \Throwable
      */
-    public function delete(AbstractEntity &$entity): void
+    public function delete(AbstractEntity $entity): void
     {
         $this->config->checkEntity($entity);
         if ($this->config->hasSoftDelete()) {
@@ -144,8 +148,12 @@ public function delete(AbstractEntity &$entity): void
                 $this->save($entity);
             }
         } else {
-            $where = $this->getPkCondition($entity);
-            $this->getConnection()->delete($this->getTableName(), $where);
+            $whereParams = $this->getPkCondition($entity);
+            $whereString = implode(' AND ', array_map(fn ($key) => $this->escapeIdentifier($key) . "=?", array_keys($whereParams)));
+            $this->getConnection()->executeQuery(
+                sql: "DELETE FROM " . $this->escapeIdentifier($this->getTableName()) . " WHERE $whereString;",
+                params: array_values($whereParams),
+            );
         }
     }
 
@@ -192,8 +200,15 @@ protected function _countAll(array|Where $where = []): int
      */
     protected function _findByPk(mixed $pk): mixed
     {
-        $where = $this->getPkCondition($pk);
-        return $this->_findOne($where);
+        $whereParams = $this->getPkCondition($pk);
+        $whereString = implode(' AND ', array_map(fn ($key) => $this->escapeIdentifier($key) . "=?", array_keys($whereParams)));
+        $row = $this->getConnection()
+            ->executeQuery(
+                sql: "SELECT * FROM " . $this->escapeIdentifier($this->getTableName()) . " WHERE $whereString;",
+                params: array_values($whereParams),
+            )
+            ->fetchAssociative();
+        return $this->createEntity($row);
     }
 
     /**
@@ -304,7 +319,14 @@ protected function getPkCondition(int|string|array|AbstractEntity|UuidInterface
     {
         $condition = [];
         if ($data instanceof AbstractEntity) {
-            $data = $data->toArray();
+            if ($data->isNew()) {
+                $data = $data->toArray();
+            } else {
+                foreach ($this->config->primaryKeys as $key) {
+                    $condition[$key] = $data->getOldValue($key);
+                }
+                return $condition;
+            }
         }
         if (is_array($data)) {
             foreach ($this->config->primaryKeys as $key) {
@@ -324,20 +346,4 @@ private function checkUpdatedAt(AbstractEntity $entity): void
             $entity->updated_at = new \DateTimeImmutable();
         }
     }
-
-    /**
-     * @param array<string, mixed> $data
-     * @return array<string, mixed>
-     * @throws \Doctrine\DBAL\Exception
-     */
-    private function formatData(array $data): array
-    {
-        $supportsBoolean = $this->getConnection()->getDatabasePlatform() instanceof PostgreSQLPlatform;
-        foreach ($data as $columnName => $value) {
-            if (is_bool($value) && !$supportsBoolean) {
-                $data[$columnName] = $value ? 1 : 0;
-            }
-        }
-        return $data;
-    }
 }
diff --git a/src/Helpers/DatabaseSpecificTrait.php b/src/Helpers/DatabaseSpecificTrait.php
@@ -0,0 +1,60 @@
+<?php declare(strict_types=1);
+
+namespace Composite\DB\Helpers;
+
+use Composite\DB\Exceptions\DbException;
+use Doctrine\DBAL\Driver;
+
+trait DatabaseSpecificTrait
+{
+    private ?bool $isPostgreSQL = null;
+    private ?bool $isMySQL = null;
+    private ?bool $isSQLite = null;
+
+    private function identifyPlatform(): void
+    {
+        if ($this->isPostgreSQL !== null) {
+            return;
+        }
+        $driver = $this->getConnection()->getDriver();
+        if ($driver instanceof Driver\AbstractPostgreSQLDriver) {
+            $this->isPostgreSQL = true;
+            $this->isMySQL = $this->isSQLite = false;
+        } elseif ($driver instanceof Driver\AbstractSQLiteDriver) {
+            $this->isSQLite = true;
+            $this->isPostgreSQL = $this->isMySQL = false;
+        } elseif ($driver instanceof Driver\AbstractMySQLDriver) {
+            $this->isMySQL = true;
+            $this->isPostgreSQL = $this->isSQLite = false;
+        } else {
+            // @codeCoverageIgnoreStart
+            throw new DbException('Unsupported driver ' . $driver::class);
+            // @codeCoverageIgnoreEnd
+        }
+    }
+
+    /**
+     * @param array<string, mixed> $data
+     * @return array<string, mixed>
+     */
+    private function prepareDataForSql(array $data): array
+    {
+        $this->identifyPlatform();
+        foreach ($data as $columnName => $value) {
+            if (is_bool($value) && !$this->isPostgreSQL) {
+                $data[$columnName] = $value ? 1 : 0;
+            }
+        }
+        return $data;
+    }
+
+    protected function escapeIdentifier(string $key): string
+    {
+        $this->identifyPlatform();
+        if ($this->isMySQL) {
+            return implode('.', array_map(fn ($part) => "`$part`", explode('.', $key)));
+        } else {
+            return '"' . $key . '"';
+        }
+    }
+}
diff --git a/src/SelectRawTrait.php → src/Helpers/SelectRawTrait.php b/src/SelectRawTrait.php → src/Helpers/SelectRawTrait.php
@@ -1,7 +1,8 @@
 <?php declare(strict_types=1);
 
-namespace Composite\DB;
+namespace Composite\DB\Helpers;
 
+use Composite\DB\Where;
 use Doctrine\DBAL\Query\QueryBuilder;
 
 trait SelectRawTrait

diff --git a/src/MultiQuery/MultiInsert.php b/src/MultiQuery/MultiInsert.php
@@ -2,8 +2,14 @@
 
 namespace Composite\DB\MultiQuery;
 
+use Composite\DB\Helpers\DatabaseSpecificTrait;
+use Doctrine\DBAL\Connection;
+
 class MultiInsert
 {
+    use DatabaseSpecificTrait;
+
+    private Connection $connection;
     private string $sql = '';
     /** @var array<string, mixed> */
     private array $parameters = [];
@@ -12,13 +18,14 @@ class MultiInsert
      * @param string $tableName
      * @param list<array<string, mixed>> $rows
      */
-    public function __construct(string $tableName, array $rows) {
+    public function __construct(Connection $connection, string $tableName, array $rows) {
         if (!$rows) {
             return;
         }
+        $this->connection = $connection;
         $firstRow = reset($rows);
-        $columnNames = array_map(fn($columnName) => "`$columnName`", array_keys($firstRow));
-        $this->sql = "INSERT INTO `$tableName` (" . implode(', ', $columnNames) . ") VALUES ";
+        $columnNames = array_map(fn ($columnName) => $this->escapeIdentifier($columnName), array_keys($firstRow));
+        $this->sql = "INSERT INTO " . $this->escapeIdentifier($tableName)  . " (" . implode(', ', $columnNames) . ") VALUES ";
         $valuesSql = [];
 
         $index = 0;
@@ -47,4 +54,9 @@ public function getParameters(): array
     {
         return $this->parameters;
     }
+
+    private function getConnection(): Connection
+    {
+        return $this->connection;
+    }
 }
diff --git a/tests/MultiQuery/MultiInsertTest.php b/tests/MultiQuery/MultiInsertTest.php
@@ -2,6 +2,7 @@
 
 namespace Composite\DB\Tests\MultiQuery;
 
+use Composite\DB\ConnectionManager;
 use Composite\DB\MultiQuery\MultiInsert;
 
 class MultiInsertTest extends \PHPUnit\Framework\TestCase
@@ -11,7 +12,8 @@ class MultiInsertTest extends \PHPUnit\Framework\TestCase
      */
     public function test_multiInsertQuery($tableName, $rows, $expectedSql, $expectedParameters)
     {
-        $multiInserter = new MultiInsert($tableName, $rows);
+        $connection = ConnectionManager::getConnection('sqlite');
+        $multiInserter = new MultiInsert($connection, $tableName, $rows);
 
         $this->assertEquals($expectedSql, $multiInserter->getSql());
         $this->assertEquals($expectedParameters, $multiInserter->getParameters());
@@ -31,7 +33,7 @@ public static function multiInsertQuery_dataProvider()
                 [
                     ['a' => 'value1_1', 'b' => 'value2_1'],
                 ],
-                "INSERT INTO `testTable` (`a`, `b`) VALUES (:a0, :b0);",
+                'INSERT INTO "testTable" ("a", "b") VALUES (:a0, :b0);',
                 ['a0' => 'value1_1', 'b0' => 'value2_1']
             ],
             [
@@ -40,7 +42,7 @@ public static function multiInsertQuery_dataProvider()
                     ['a' => 'value1_1', 'b' => 'value2_1'],
                     ['a' => 'value1_2', 'b' => 'value2_2']
                 ],
-                "INSERT INTO `testTable` (`a`, `b`) VALUES (:a0, :b0), (:a1, :b1);",
+                'INSERT INTO "testTable" ("a", "b") VALUES (:a0, :b0), (:a1, :b1);',
                 ['a0' => 'value1_1', 'b0' => 'value2_1', 'a1' => 'value1_2', 'b1' => 'value2_2']
             ],
             [
@@ -49,7 +51,7 @@ public static function multiInsertQuery_dataProvider()
                     ['column1' => 'value1_1'],
                     ['column1' => 123]
                 ],
-                "INSERT INTO `testTable` (`column1`) VALUES (:column10), (:column11);",
+                'INSERT INTO "testTable" ("column1") VALUES (:column10), (:column11);',
                 ['column10' => 'value1_1', 'column11' => 123]
             ]
         ];