From d37a4729e1302b12e1b2f548a9c610ed2a1c2212 Mon Sep 17 00:00:00 2001
From: Andreas Fernandez <andreas.fernandez@aspedia.de>
Date: Mon, 24 Nov 2014 17:29:48 +0100
Subject: [PATCH] [BUGFIX] Introduce chunking for large expression lists

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>
---
 .../Classes/Database/DatabaseConnection.php   | 26 +++++++
 .../Database/Specifics/AbstractSpecifics.php  | 71 +++++++++++++++++++
 .../dbal/Classes/Database/Specifics/Oci8.php  | 32 +++++++++
 .../dbal/Classes/Database/SqlParser.php       | 40 ++++++++++-
 .../Database/DatabaseConnectionOracleTest.php | 63 ++++++++++++++++
 .../Domain/Repository/ExtensionRepository.php | 28 ++------
 6 files changed, 237 insertions(+), 23 deletions(-)
 create mode 100644 typo3/sysext/dbal/Classes/Database/Specifics/AbstractSpecifics.php
 create mode 100644 typo3/sysext/dbal/Classes/Database/Specifics/Oci8.php

diff --git a/typo3/sysext/dbal/Classes/Database/DatabaseConnection.php b/typo3/sysext/dbal/Classes/Database/DatabaseConnection.php
index c75de074e836..683803b67f00 100644
--- a/typo3/sysext/dbal/Classes/Database/DatabaseConnection.php
+++ b/typo3/sysext/dbal/Classes/Database/DatabaseConnection.php
@@ -202,6 +202,11 @@ class DatabaseConnection extends \TYPO3\CMS\Core\Database\DatabaseConnection {
 		MYSQLI_TYPE_GEOMETRY => 'geometry'
 	);
 
+	/**
+	 * @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:
@@ -237,6 +254,15 @@ class DatabaseConnection extends \TYPO3\CMS\Core\Database\DatabaseConnection {
 		$this->debug = !empty($this->conf['debugOptions']['enabled']);
 	}
 
+	/**
+	 * Gets the DBMS specifics object
+	 *
+	 * @return Specifics\AbstractSpecifics
+	 */
+	public function getSpecifics() {
+		return $this->dbmsSpecifics;
+	}
+
 	/**
 	 * @return \TYPO3\CMS\Core\Cache\Frontend\PhpFrontend
 	 */
diff --git a/typo3/sysext/dbal/Classes/Database/Specifics/AbstractSpecifics.php b/typo3/sysext/dbal/Classes/Database/Specifics/AbstractSpecifics.php
new file mode 100644
index 000000000000..7fb7e3a76668
--- /dev/null
+++ b/typo3/sysext/dbal/Classes/Database/Specifics/AbstractSpecifics.php
@@ -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
index 000000000000..2d4ab19c708e
--- /dev/null
+++ b/typo3/sysext/dbal/Classes/Database/Specifics/Oci8.php
@@ -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
diff --git a/typo3/sysext/dbal/Classes/Database/SqlParser.php b/typo3/sysext/dbal/Classes/Database/SqlParser.php
index b9dbcfbd1596..3100541c86bc 100644
--- a/typo3/sysext/dbal/Classes/Database/SqlParser.php
+++ b/typo3/sysext/dbal/Classes/Database/SqlParser.php
@@ -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];
diff --git a/typo3/sysext/dbal/Tests/Unit/Database/DatabaseConnectionOracleTest.php b/typo3/sysext/dbal/Tests/Unit/Database/DatabaseConnectionOracleTest.php
index 2bd5bd216f53..3b7a241a61be 100644
--- a/typo3/sysext/dbal/Tests/Unit/Database/DatabaseConnectionOracleTest.php
+++ b/typo3/sysext/dbal/Tests/Unit/Database/DatabaseConnectionOracleTest.php
@@ -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
diff --git a/typo3/sysext/extensionmanager/Classes/Domain/Repository/ExtensionRepository.php b/typo3/sysext/extensionmanager/Classes/Domain/Repository/ExtensionRepository.php
index 6f37bef2c719..9ddbdf9fa838 100644
--- a/typo3/sysext/extensionmanager/Classes/Domain/Repository/ExtensionRepository.php
+++ b/typo3/sysext/extensionmanager/Classes/Domain/Repository/ExtensionRepository.php
@@ -25,15 +25,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
 	 */
@@ -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,
+			)
+		);
 	}
 
 	/**
-- 
GitLab