[BUGFIX] Introduce chunking for large expression lists 58/34558/9
authorAndreas Fernandez <andreas.fernandez@aspedia.de>
Mon, 24 Nov 2014 16:29:48 +0000 (17:29 +0100)
committerMarkus Klein <klein.t3@reelworx.at>
Wed, 26 Nov 2014 09:43:10 +0000 (10:43 +0100)
TYPO3 executes some queries that contain very large expression lists,
e.g. in "NOT IN". In Oracle, this actually fails because the amount
of items in those lists is limited.

The code is prepared to support more specifics in different DBMS
at any time.

This patch also reverts If63f855b250bf7c9b6cd7112f60392cfc8ccfd67
because it's obsolete now.

Resolves: #61654
Related: #60859
Releases: master, 6.2
Change-Id: I3afd6a5187f28a9ddd7b01947e278fc7ce853808
Reviewed-on: http://review.typo3.org/34558
Reviewed-by: Xavier Perseguers <xavier@typo3.org>
Tested-by: Xavier Perseguers <xavier@typo3.org>
Reviewed-by: Markus Klein <klein.t3@reelworx.at>
Tested-by: Markus Klein <klein.t3@reelworx.at>
typo3/sysext/dbal/Classes/Database/DatabaseConnection.php
typo3/sysext/dbal/Classes/Database/Specifics/AbstractSpecifics.php [new file with mode: 0644]
typo3/sysext/dbal/Classes/Database/Specifics/Oci8.php [new file with mode: 0644]
typo3/sysext/dbal/Classes/Database/SqlParser.php
typo3/sysext/dbal/Tests/Unit/Database/DatabaseConnectionOracleTest.php
typo3/sysext/extensionmanager/Classes/Domain/Repository/ExtensionRepository.php

index c75de07..683803b 100644 (file)
@@ -203,6 +203,11 @@ class DatabaseConnection extends \TYPO3\CMS\Core\Database\DatabaseConnection {
        );
 
        /**
+        * @var Specifics\AbstractSpecifics
+        */
+       protected $dbmsSpecifics;
+
+       /**
         * Constructor.
         * Creates SQL parser object and imports configuration from $TYPO3_CONF_VARS['EXTCONF']['dbal']
         */
@@ -230,6 +235,18 @@ class DatabaseConnection extends \TYPO3\CMS\Core\Database\DatabaseConnection {
                }
                if (isset($this->conf['handlerCfg'])) {
                        $this->handlerCfg = $this->conf['handlerCfg'];
+
+                       if (isset($this->handlerCfg['_DEFAULT']['config']['driver'])) {
+                               // load DBMS specifics
+                               $driver = $this->handlerCfg['_DEFAULT']['config']['driver'];
+                               $className = 'TYPO3\\CMS\\Dbal\\Database\\Specifics\\' . ucfirst(strtolower($driver));
+                               if (class_exists($className)) {
+                                       if (!is_subclass_of($className, Specifics\AbstractSpecifics::class)) {
+                                               throw new \InvalidArgumentException($className . ' must inherit from ' . Specifics\AbstractSpecifics::class, 1416919866);
+                                       }
+                                       $this->dbmsSpecifics = GeneralUtility::makeInstance($className);
+                               }
+                       }
                }
                $this->cacheFieldInfo();
                // Debugging settings:
@@ -238,6 +255,15 @@ class DatabaseConnection extends \TYPO3\CMS\Core\Database\DatabaseConnection {
        }
 
        /**
+        * Gets the DBMS specifics object
+        *
+        * @return Specifics\AbstractSpecifics
+        */
+       public function getSpecifics() {
+               return $this->dbmsSpecifics;
+       }
+
+       /**
         * @return \TYPO3\CMS\Core\Cache\Frontend\PhpFrontend
         */
        protected function getFieldInfoCache() {
diff --git a/typo3/sysext/dbal/Classes/Database/Specifics/AbstractSpecifics.php b/typo3/sysext/dbal/Classes/Database/Specifics/AbstractSpecifics.php
new file mode 100644 (file)
index 0000000..7fb7e3a
--- /dev/null
@@ -0,0 +1,71 @@
+<?php
+namespace TYPO3\CMS\Dbal\Database\Specifics;
+
+/**
+ * This file is part of the TYPO3 CMS project.
+ *
+ * It is free software; you can redistribute it and/or modify it under
+ * the terms of the GNU General Public License, either version 2
+ * of the License, or any later version.
+ *
+ * For the full copyright and license information, please read the
+ * LICENSE.txt file that was distributed with this source code.
+ *
+ * The TYPO3 project - inspiring people to share!
+ */
+
+/**
+ * This class handles the specifics of the active DBMS. Inheriting classes
+ * are intended to define their own specifics.
+ */
+abstract class AbstractSpecifics {
+       /**
+        * Constants used as identifiers in $specificProperties.
+        */
+       const TABLE_MAXLENGTH = 'table_maxlength';
+       const FIELD_MAXLENGTH = 'field_maxlength';
+       const LIST_MAXEXPRESSIONS = 'list_maxexpressions';
+
+       /**
+        * Contains the specifics of a DBMS.
+        * This is intended to be overridden by inheriting classes.
+        *
+        * @var array
+        */
+       protected $specificProperties = array();
+
+       /**
+        * Checks if a specific is defined for the used DBMS.
+        *
+        * @param string $specific
+        * @return bool
+        */
+       public function specificExists($specific) {
+               return isset($this->specificProperties[$specific]);
+       }
+
+       /**
+        * Gets the specific value.
+        *
+        * @param string $specific
+        * @return mixed
+        */
+       public function getSpecific($specific) {
+               return $this->specificProperties[$specific];
+       }
+
+       /**
+        * Splits $expressionList into multiple chunks.
+        *
+        * @param array $expressionList
+        * @param bool $preserveArrayKeys If TRUE, array keys are preserved in array_chunk()
+        * @return array
+        */
+       public function splitMaxExpressions($expressionList, $preserveArrayKeys = FALSE) {
+               if (!$this->specificExists(self::LIST_MAXEXPRESSIONS)) {
+                       return array($expressionList);
+               }
+
+               return array_chunk($expressionList, $this->getSpecific(self::LIST_MAXEXPRESSIONS), $preserveArrayKeys);
+       }
+}
\ No newline at end of file
diff --git a/typo3/sysext/dbal/Classes/Database/Specifics/Oci8.php b/typo3/sysext/dbal/Classes/Database/Specifics/Oci8.php
new file mode 100644 (file)
index 0000000..2d4ab19
--- /dev/null
@@ -0,0 +1,32 @@
+<?php
+namespace TYPO3\CMS\Dbal\Database\Specifics;
+
+/**
+ * This file is part of the TYPO3 CMS project.
+ *
+ * It is free software; you can redistribute it and/or modify it under
+ * the terms of the GNU General Public License, either version 2
+ * of the License, or any later version.
+ *
+ * For the full copyright and license information, please read the
+ * LICENSE.txt file that was distributed with this source code.
+ *
+ * The TYPO3 project - inspiring people to share!
+ */
+
+/**
+ * This class contains the specifics for Oracle DBMS.
+ * Any logic is in AbstractSpecifics.
+ */
+class Oci8 extends AbstractSpecifics {
+       /**
+        * Contains the specifics that need to be taken care of for Oracle DBMS.
+        *
+        * @var array
+        */
+       protected $specificProperties = array(
+               self::TABLE_MAXLENGTH => 30,
+               self::FIELD_MAXLENGTH => 30,
+               self::LIST_MAXEXPRESSIONS => 1000
+       );
+}
\ No newline at end of file
index b9dbcfb..3100541 100644 (file)
@@ -654,7 +654,8 @@ class SqlParser extends \TYPO3\CMS\Core\Database\SqlParser {
                                                                                }
                                                                                $output .= ' ' . $v['comparator'];
                                                                                // Detecting value type; list or plain:
-                                                                               if (GeneralUtility::inList('NOTIN,IN', strtoupper(str_replace(array(' ', TAB, CR, LF), '', $v['comparator'])))) {
+                                                                               $comparator = strtoupper(str_replace(array(' ', TAB, CR, LF), '', $v['comparator']));
+                                                                               if (GeneralUtility::inList('NOTIN,IN', $comparator)) {
                                                                                        if (isset($v['subquery'])) {
                                                                                                $output .= ' (' . $this->compileSELECT($v['subquery']) . ')';
                                                                                        } else {
@@ -662,7 +663,42 @@ class SqlParser extends \TYPO3\CMS\Core\Database\SqlParser {
                                                                                                foreach ($v['value'] as $realValue) {
                                                                                                        $valueBuffer[] = $realValue[1] . $this->compileAddslashes($realValue[0]) . $realValue[1];
                                                                                                }
-                                                                                               $output .= ' (' . trim(implode(',', $valueBuffer)) . ')';
+
+                                                                                               $dbmsSpecifics = $this->databaseConnection->getSpecifics();
+                                                                                               if ($dbmsSpecifics === NULL) {
+                                                                                                       $output .= ' (' . trim(implode(',', $valueBuffer)) . ')';
+                                                                                               } else {
+                                                                                                       $chunkedList = $dbmsSpecifics->splitMaxExpressions($valueBuffer);
+                                                                                                       $chunkCount = count($chunkedList);
+
+                                                                                                       if ($chunkCount === 1) {
+                                                                                                               $output .= ' (' . trim(implode(',', $valueBuffer)) . ')';
+                                                                                                       } else {
+                                                                                                               $listExpressions = array();
+                                                                                                               $field = trim(($v['table'] ? $v['table'] . '.' : '') . $v['field']);
+
+                                                                                                               switch ($comparator) {
+                                                                                                                       case 'IN':
+                                                                                                                               $operator = 'OR';
+                                                                                                                               break;
+                                                                                                                       case 'NOTIN':
+                                                                                                                               $operator = 'AND';
+                                                                                                                               break;
+                                                                                                               }
+
+                                                                                                               for ($i = 0; $i < $chunkCount; ++$i) {
+                                                                                                                       $listPart = trim(implode(',', $chunkedList[$i]));
+                                                                                                                       $listExpressions[] = ' (' . $listPart . ')';
+                                                                                                               }
+
+                                                                                                               $implodeString = ' ' . $operator . ' ' . $field . ' ' . $v['comparator'];
+
+                                                                                                               // add opening brace before field
+                                                                                                               $lastFieldPos = strpos($output, $field);
+                                                                                                               $output = substr_replace($output, '(', $lastFieldPos, 0);
+                                                                                                               $output .= implode($implodeString, $listExpressions) . ')';
+                                                                                                       }
+                                                                                               }
                                                                                        }
                                                                                } elseif (GeneralUtility::inList('BETWEEN,NOT BETWEEN', $v['comparator'])) {
                                                                                        $lbound = $v['values'][0];
index 2bd5bd2..3b7a241 100644 (file)
@@ -909,4 +909,67 @@ class DatabaseConnectionOracleTest extends AbstractTestCase {
                $expected = 'SELECT * FROM "tt_content" WHERE (dbms_lob.instr("bodytext", \'test\',1,1) > 0)';
                $this->assertEquals($expected, $this->cleanSql($result));
        }
+
+       /**
+        * @test
+        */
+       public function expressionListWithNotInIsConcatenatedWithAnd() {
+               $listMaxExpressions = 1000;
+
+               $mockSpecificsOci8 = $this->getAccessibleMock('TYPO3\\CMS\\Dbal\\Database\\Specifics\\Oci8', array(), array(), '', FALSE);
+               $mockSpecificsOci8->expects($this->any())->method('getSpecific')->will($this->returnValue($listMaxExpressions));
+
+               $items = range(0, 1250);
+               $where = 'uid NOT IN(' . implode(',', $items) . ')';
+               $result = $this->subject->SELECTquery('*', 'tt_content', $where);
+
+               $chunks = array_chunk($items, $listMaxExpressions);
+               $whereExpr = array();
+               foreach ($chunks as $chunk) {
+                       $whereExpr[] = '"uid" NOT IN (' . implode(',', $chunk) . ')';
+               }
+
+               $expectedWhere = '(' . implode(' AND ', $whereExpr) . ')';
+               $expectedQuery = 'SELECT * FROM "tt_content" WHERE ' . $expectedWhere;
+               $this->assertEquals($expectedQuery, $this->cleanSql($result));
+       }
+
+       /**
+        * @test
+        */
+       public function expressionListWithInIsConcatenatedWithOr() {
+               $listMaxExpressions = 1000;
+
+               $mockSpecificsOci8 = $this->getAccessibleMock('TYPO3\\CMS\\Dbal\\Database\\Specifics\\Oci8', array(), array(), '', FALSE);
+               $mockSpecificsOci8->expects($this->any())->method('getSpecific')->will($this->returnValue($listMaxExpressions));
+
+               $items = range(0, 1250);
+               $where = 'uid IN(' . implode(',', $items) . ')';
+               $result = $this->subject->SELECTquery('*', 'tt_content', $where);
+
+               $chunks = array_chunk($items, $listMaxExpressions);
+               $whereExpr = array();
+               foreach ($chunks as $chunk) {
+                       $whereExpr[] = '"uid" IN (' . implode(',', $chunk) . ')';
+               }
+
+               $expectedWhere = '(' . implode(' OR ', $whereExpr) . ')';
+               $expectedQuery = 'SELECT * FROM "tt_content" WHERE ' . $expectedWhere;
+               $this->assertEquals($expectedQuery, $this->cleanSql($result));
+       }
+
+       /**
+        * @test
+        */
+       public function expressionListIsUnchanged() {
+               $listMaxExpressions = 1000;
+
+               $mockSpecificsOci8 = $this->getAccessibleMock('TYPO3\\CMS\\Dbal\\Database\\Specifics\\Oci8', array(), array(), '', FALSE);
+               $mockSpecificsOci8->expects($this->any())->method('getSpecific')->will($this->returnValue($listMaxExpressions));
+
+               $result = $this->subject->SELECTquery('*', 'tt_content', 'uid IN (0,1,2,3,4,5,6,7,8,9,10)');
+
+               $expectedQuery = 'SELECT * FROM "tt_content" WHERE "uid" IN (0,1,2,3,4,5,6,7,8,9,10)';
+               $this->assertEquals($expectedQuery, $this->cleanSql($result));
+       }
 }
\ No newline at end of file
index 6f37bef..9ddbdf9 100644 (file)
@@ -26,15 +26,6 @@ class ExtensionRepository extends \TYPO3\CMS\Extbase\Persistence\Repository {
        const TABLE_NAME = 'tx_extensionmanager_domain_model_extension';
 
        /**
-        * Oracle has a limit of 1000 values in an IN clause. Set the size of a chunk
-        * being updated to 500 to make sure it does not collide with a limit in any
-        * other DBMS.
-        *
-        * @var int
-        */
-       const CHUNK_SIZE = 500;
-
-       /**
         * @var \TYPO3\CMS\Core\Database\DatabaseConnection
         */
        protected $databaseConnection;
@@ -293,18 +284,13 @@ class ExtensionRepository extends \TYPO3\CMS\Extbase\Persistence\Repository {
        protected function markExtensionWithMaximumVersionAsCurrent($repositoryUid) {
                $uidsOfCurrentVersion = $this->fetchMaximalVersionsForAllExtensions($repositoryUid);
 
-               // some DBMS limit the amount of expressions, apply the update in chunks
-               $chunks = array_chunk($uidsOfCurrentVersion, self::CHUNK_SIZE);
-               $chunkCount = count($chunks);
-               for ($i = 0; $i < $chunkCount; ++$i) {
-                       $this->databaseConnection->exec_UPDATEquery(
-                               self::TABLE_NAME,
-                               'uid IN (' . implode(',', $chunks[$i]) . ')',
-                               array(
-                                       'current_version' => 1,
-                               )
-                       );
-               }
+               $this->databaseConnection->exec_UPDATEquery(
+                       self::TABLE_NAME,
+                       'uid IN (' . implode(',', $uidsOfCurrentVersion) . ')',
+                       array(
+                               'current_version' => 1,
+                       )
+               );
        }
 
        /**