[TASK] Handle some sqlite details 09/57209/7
authorChristian Kuhn <lolli@schwarzbu.ch>
Wed, 13 Jun 2018 11:47:48 +0000 (13:47 +0200)
committerBenni Mack <benni@typo3.org>
Wed, 13 Jun 2018 16:15:21 +0000 (18:15 +0200)
* Similar to postgresql, sqlite index names must be unique
  within the entire database. The patch adds a hash of the
  table name in front of indexes to make them unique.

* SELECT'ing rows from a table and UPDATE'ing them while the
  select query is still running is not safe in sqlite, single
  rows may appear over and over again in the select() result
  set. The patch switches a query combination to a fetchAll()
  on sqlite platform to prevent this.

Change-Id: Ib35ab4f46bbce7867ff9e4624e545b505c4f5e57
Resolves: #85253
Releases: master
Reviewed-on: https://review.typo3.org/57209
Reviewed-by: Andreas Fernandez <a.fernandez@scripting-base.de>
Tested-by: Andreas Fernandez <a.fernandez@scripting-base.de>
Tested-by: TYPO3com <no-reply@typo3.com>
Reviewed-by: Benni Mack <benni@typo3.org>
Tested-by: Benni Mack <benni@typo3.org>
typo3/sysext/core/Classes/DataHandling/DataHandler.php
typo3/sysext/core/Classes/Database/Schema/ConnectionMigrator.php
typo3/sysext/core/Tests/Functional/Database/Schema/SchemaMigratorTest.php

index f740a74..3247b28 100644 (file)
@@ -17,6 +17,7 @@ namespace TYPO3\CMS\Core\DataHandling;
 use Doctrine\DBAL\DBALException;
 use Doctrine\DBAL\Driver\Statement;
 use Doctrine\DBAL\Platforms\PostgreSqlPlatform;
+use Doctrine\DBAL\Platforms\SqlitePlatform;
 use Doctrine\DBAL\Platforms\SQLServerPlatform;
 use Doctrine\DBAL\Types\IntegerType;
 use Psr\Log\LoggerAwareInterface;
@@ -7485,19 +7486,45 @@ class DataHandler implements LoggerAwareInterface
                 ->orderBy($sortRow, 'ASC')
                 ->addOrderBy('uid', 'ASC')
                 ->execute();
-            while ($row = $result->fetch()) {
-                $uid = (int)$row['uid'];
-                if ($uid) {
-                    $connection->update($table, [$sortRow => $i], ['uid' => (int)$uid]);
-                    // This is used to return a sortingValue if the list is resorted because of inserting records inside the list and not in the top
-                    if ($uid == $return_SortNumber_After_This_Uid) {
-                        $i += $intervals;
-                        $returnVal = $i;
+            if ($connection->getDatabasePlatform() instanceof SqlitePlatform) {
+                // The default iteration behavior "fetch single row and update it" below can fail on sqlite.
+                // See https://bugs.php.net/bug.php?id=72267 and https://www.sqlite.org/isolation.html for details, money quote:
+                // "If changes occur on the same database connection after a query starts running but before the query completes,
+                // then the query might return a changed row more than once, or it might return a row that was previously deleted."
+                // In this resorting case, sqlite tends to run into infinite loops by returning the same rows over and over again
+                // with their already updated sorting values. As less memory efficient but safe solution, we just fetchAll() all
+                // rows into an array and update them in a second step.
+                $result = $result->fetchAll();
+                foreach ($result as $row) {
+                    $uid = (int)$row['uid'];
+                    if ($uid) {
+                        $connection->update($table, [$sortRow => $i], ['uid' => (int)$uid]);
+                        // This is used to return a sortingValue if the list is resorted because of inserting records inside the list and not in the top
+                        if ($uid == $return_SortNumber_After_This_Uid) {
+                            $i += $intervals;
+                            $returnVal = $i;
+                        }
+                    } else {
+                        die('Fatal ERROR!! No Uid at resorting.');
                     }
-                } else {
-                    die('Fatal ERROR!! No Uid at resorting.');
+                    $i += $intervals;
+                }
+            } else {
+                while ($row = $result->fetch()) {
+                    // fetch() and update() single rows in one go
+                    $uid = (int)$row['uid'];
+                    if ($uid) {
+                        $connection->update($table, [$sortRow => $i], ['uid' => (int)$uid]);
+                        // This is used to return a sortingValue if the list is resorted because of inserting records inside the list and not in the top
+                        if ($uid == $return_SortNumber_After_This_Uid) {
+                            $i += $intervals;
+                            $returnVal = $i;
+                        }
+                    } else {
+                        die('Fatal ERROR!! No Uid at resorting.');
+                    }
+                    $i += $intervals;
                 }
-                $i += $intervals;
             }
             return $returnVal;
         }
index f99881d..990fb6c 100644 (file)
@@ -18,6 +18,7 @@ namespace TYPO3\CMS\Core\Database\Schema;
 use Doctrine\DBAL\DBALException;
 use Doctrine\DBAL\Platforms\MySqlPlatform;
 use Doctrine\DBAL\Platforms\PostgreSqlPlatform;
+use Doctrine\DBAL\Platforms\SqlitePlatform;
 use Doctrine\DBAL\Schema\Column;
 use Doctrine\DBAL\Schema\ColumnDiff;
 use Doctrine\DBAL\Schema\ForeignKeyConstraint;
@@ -164,7 +165,9 @@ class ConnectionMigrator
                             $columnName = preg_replace('/\(\d+\)$/', '', $columnName);
                             // Strip mssql '[' and ']' from column names
                             $columnName = ltrim($columnName, '[');
-                            return rtrim($columnName, ']');
+                            $columnName = rtrim($columnName, ']');
+                            // Strip sqlite '"' from column names
+                            return trim($columnName, '"');
                         },
                         $addedIndex->getColumns()
                     );
@@ -1098,8 +1101,10 @@ class ConnectionMigrator
             $indexes = [];
             foreach ($table->getIndexes() as $key => $index) {
                 $indexName = $index->getName();
-                // PostgreSQL requires index names to be unique per database/schema.
-                if ($connection->getDatabasePlatform() instanceof PostgreSqlPlatform) {
+                // PostgreSQL and sqlite require index names to be unique per database/schema.
+                if ($connection->getDatabasePlatform() instanceof PostgreSqlPlatform
+                    || $connection->getDatabasePlatform() instanceof SqlitePlatform
+                ) {
                     $indexName = $indexName . '_' . hash('crc32b', $table->getName() . '_' . $indexName);
                 }
 
index 832b62e..bc3c208 100644 (file)
@@ -310,7 +310,6 @@ class SchemaMigratorTest extends FunctionalTestCase
      * probably be enabled.
      *
      * @test
-     * @group not-sqlite
      */
     public function installDoesNotAddIndexOnChangedColumn()
     {