[TASK] Handle some sqlite details

* 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

@@ -17,6 +17,7 @@
 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 @@ public function resorting($table, $pid, $sortRow, $return_SortNumber_After_This_
                 ->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;
         }

typo3/sysext/core/Classes/Database/Schema/ConnectionMigrator.php

@@ -18,6 +18,7 @@
 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 @@ function ($columnName) {
                             $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 @@ protected function transformTablesForDatabasePlatform(array $tables, Connection
             $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);
                 }
 

typo3/sysext/core/Tests/Functional/Database/Schema/SchemaMigratorTest.php

@@ -310,7 +310,6 @@ public function installPerformsOnlyAddAndCreateOperations()
      * probably be enabled.
      *
      * @test
-     * @group not-sqlite
      */
     public function installDoesNotAddIndexOnChangedColumn()
     {