fix(db): only cast text and binary compare columns to char on Oracle

changelog/unreleased/41782

@@ -0,0 +1,14 @@
+Bugfix: Restore index usage for filecache writes on Oracle
+
+On Oracle every compare column of an upsert was wrapped in to_char(). That cast
+is only needed for text and binary columns, which Oracle cannot compare
+directly, but it was applied to all of them - and to_char(column) cannot use an
+index on that column. Writes to the file cache compare storage and path_hash,
+so uploads, renames and file scans could no longer use the unique index
+fs_storage_path_hash and became very slow on large installations.
+
+Only text and binary compare columns are cast now, so every other comparison
+uses its index again.
+
+https://github.com/owncloud/core/issues/41782
+https://github.com/owncloud/core/pull/41783

lib/private/DB/Adapter.php

@@ -115,22 +115,44 @@ public function insertIfNotExist($table, $input, array $compare = null) {
 		return $this->conn->executeUpdate($query, $inserts);
 	}
 
+	/**
+	 * Types to pass to the expression builder for the compare columns, keyed by
+	 * column name. Only platforms that need to treat some column types
+	 * differently in a comparison return anything here - see AdapterOCI8.
+	 *
+	 * A column that is missing from the result is compared as it is, which is
+	 * what every platform except Oracle does with any type anyway, because
+	 * ExpressionBuilder::eq() ignores its type argument.
+	 *
+	 * @param string $table table name including **PREFIX**
+	 * @param string[] $compare columns that are compared to look for existing rows
+	 * @return array column name => one of \OCP\DB\QueryBuilder\IQueryBuilder::PARAM_*
+	 */
+	protected function getCompareColumnTypes($table, array $compare) {
+		return [];
+	}
+
 	/**
 	 * Inserts, or updates a row into the database. Returns the inserted or updated rows
 	 * @param $table string table name including **PREFIX**
 	 * @param $input array the key=>value pairs to insert into the db row
 	 * @param $compare array columns that should be compared to look for existing arrays
+	 *				If this is null or an empty array, all keys of $input will be compared
 	 * @return int the number of rows affected by the operation
 	 * @throws DriverException|\RuntimeException
 	 */
 	public function upsert($table, $input, $compare) {
-		$this->conn->beginTransaction();
-		$done = false;
-
 		if (empty($compare)) {
 			$compare = \array_keys($input);
 		}
 
+		// resolved before the transaction is opened, because it may query the schema
+		$compareTypes = $this->getCompareColumnTypes($table, $compare);
+		$isOracle = $this->conn->getDatabasePlatform() instanceof OraclePlatform;
+
+		$this->conn->beginTransaction();
+		$done = false;
+
 		// Construct the update query
 		$qbu = $this->conn->getQueryBuilder();
 		$qbu->update($table);
@@ -139,25 +161,19 @@ public function upsert($table, $input, $compare) {
 			->setParameter($col, $val);
 		}
 		foreach ($compare as $key) {
-			if ($input[$key] === null || ($input[$key] === '' && $this->conn->getDatabasePlatform() instanceof OraclePlatform)) {
+			if ($input[$key] === null || ($input[$key] === '' && $isOracle)) {
 				$qbu->andWhere($qbu->expr()->isNull($key));
 			} else {
-				if ($this->conn->getDatabasePlatform() instanceof OraclePlatform) {
-					$qbu->andWhere(
-						$qbu->expr()->eq(
-							// needs to cast to char in order to compare with char
-							$qbu->createFunction('to_char(`'.$key.'`)'), // TODO does this handle empty strings on oracle correctly
-							$qbu->expr()->literal($input[$key])
-						)
-					);
-				} else {
-					$qbu->andWhere(
-						$qbu->expr()->eq(
-							$key,
-							$qbu->expr()->literal($input[$key])
-						)
-					);
-				}
+				$qbu->andWhere(
+					$qbu->expr()->eq(
+						$key,
+						$qbu->expr()->literal($input[$key]),
+						// on Oracle a large object column has to be cast to char in
+						// order to be comparable at all - every other column must be
+						// left alone, or the comparison cannot use an index
+						$compareTypes[$key] ?? null
+					)
+				);
 			}
 		}
 

lib/private/DB/AdapterOCI8.php

@@ -24,7 +24,19 @@
 
 namespace OC\DB;
 
+use Doctrine\DBAL\Types\BlobType;
+use Doctrine\DBAL\Types\TextType;
+use OCP\DB\QueryBuilder\IQueryBuilder;
+
 class AdapterOCI8 extends Adapter {
+	/**
+	 * Large object columns per real table name, or null for a table whose
+	 * columns could not be resolved.
+	 *
+	 * @var array
+	 */
+	private $lobColumns = [];
+
 	public function lastInsertId($table) {
 		if ($table === null) {
 			throw new \InvalidArgumentException('Oracle requires a table name to be passed into lastInsertId()');
@@ -48,4 +60,95 @@ public function fixupStatement($statement) {
 		$statement = \str_ireplace('UNIX_TIMESTAMP()', self::UNIX_TIMESTAMP_REPLACEMENT, $statement);
 		return $statement;
 	}
+
+	/**
+	 * Oracle cannot compare a CLOB or a BLOB with `=` - the attempt fails with
+	 * ORA-00932 - so those columns have to be wrapped in `to_char()`. Every
+	 * other column has to be left alone: `to_char(col) = 'x'` is not sargable,
+	 * so the comparison cannot use an index on `col` and degrades into a scan.
+	 *
+	 * If the column types cannot be resolved, all compare columns are cast.
+	 * That is the behaviour which shipped before this distinction was made -
+	 * slow, but it can never raise ORA-00932.
+	 *
+	 * @inheritdoc
+	 */
+	protected function getCompareColumnTypes($table, array $compare) {
+		$lobColumns = $this->getLobColumns($table);
+
+		$types = [];
+		foreach ($compare as $key) {
+			if ($lobColumns === null || isset($lobColumns[$key])) {
+				$types[$key] = IQueryBuilder::PARAM_STR;
+			}
+		}
+		return $types;
+	}
+
+	/**
+	 * The names of all large object columns of the given table, as keys.
+	 *
+	 * The result is memoized per connection. A schema change within the same
+	 * request is therefore not picked up, which is acceptable: the type of an
+	 * existing column does not change underneath a running upsert.
+	 *
+	 * @param string $table table name including **PREFIX**
+	 * @return array|null null if the columns could not be resolved
+	 */
+	private function getLobColumns($table) {
+		$tableName = $this->getRealTableName($table);
+		if (\array_key_exists($tableName, $this->lobColumns)) {
+			return $this->lobColumns[$tableName];
+		}
+
+		$lobColumns = null;
+		$reason = 'the table is not known to the schema manager';
+		try {
+			// the identifier has to be quoted: ownCloud creates all tables and
+			// columns quoted, hence in lower case, while Oracle folds an unquoted
+			// identifier to upper case and would not find the table at all
+			$columns = $this->conn->getSchemaManager()->listTableColumns(
+				$this->conn->quoteIdentifier($tableName)
+			);
+			if ($columns !== []) {
+				$lobColumns = [];
+				foreach ($columns as $column) {
+					$type = $column->getType();
+					if ($type instanceof TextType || $type instanceof BlobType) {
+						// getName() and not the array key, because the key keeps the
+						// quotes of a reserved word like `oc_privatedata`.`user`
+						$lobColumns[$column->getName()] = true;
+					}
+				}
+			}
+		} catch (\Exception $e) {
+			$reason = $e->getMessage();
+		}
+
+		if ($lobColumns === null) {
+			// remember the failure as well, so this is logged once per table
+			\OC::$server->getLogger()->warning(
+				'Could not determine the column types of "{table}", falling back to comparing all columns as char: {reason}',
+				['app' => 'core', 'table' => $tableName, 'reason' => $reason]
+			);
+		}
+
+		$this->lobColumns[$tableName] = $lobColumns;
+		return $lobColumns;
+	}
+
+	/**
+	 * The table name as it exists in the database, for a name as it is passed to
+	 * the query builder. Mirrors Connection::replaceTablePrefix(), which is not
+	 * reachable from here.
+	 *
+	 * @param string $table table name including **PREFIX**
+	 * @return string
+	 */
+	private function getRealTableName($table) {
+		if (\strpos($table, '*PREFIX*') === 0) {
+			$table = \substr($table, \strlen('*PREFIX*'));
+		}
+		return $this->conn->getPrefix() . $table;
+	}
 }

lib/public/IDBConnection.php

@@ -140,7 +140,9 @@ public function insertIfNotExist($table, $input, array $compare = null);
 	 * @param array $input data that should be inserted into the table  (column name => value)
 	 * @param array|null $compare List of values that should be checked for "if not exists"
 	 *				If this is null or an empty array, all keys of $input will be compared
-	 *				Please note: text fields (clob) must not be used in the compare array
+	 *				Please note: on Oracle a text field (clob) in the compare array is
+	 *				compared as char and therefore limited to 4000 bytes, and a binary
+	 *				field (blob) cannot be compared at all
 	 * @return int number of affected rows
 	 * @throws \Doctrine\DBAL\DBALException
 	 * @since 10.0.3