Merge pull request #41051 from owncloud/preview_cleanup_fix_postgres3

jvillafanez · DeepDiver1975 · commit b523ac5fbf03 · 2023-11-17T16:20:45.000+01:00
Fix problem with postgresql complaining about a wrong cast
diff --git a/changelog/unreleased/41051 b/changelog/unreleased/41051
@@ -0,0 +1,14 @@
+Bugfix: Fix potential issue with the PreviewCleanup job in postgresql
+
+One of the filters of the preview cleanup job requires casting a filename,
+which is supposed to contain only digits, to an integer. The expected execution
+of the DB query should have filtered the results so the condition above should
+be true, but the DB's query planner might choose to apply the filters in a
+different way, so we could potentially cast random strings to integer.
+For the case of postgresql, the cast function will cause an error if the string
+can't be casted to an integer (because it has non-digit chars, for example)
+
+This situation is fixed for all the supported DBs, so we don't require the
+query planner to execute the query in any particular way.
+
+https://github.com/owncloud/core/pull/41051
diff --git a/lib/private/PreviewCleanup.php b/lib/private/PreviewCleanup.php
@@ -22,6 +22,9 @@
 
 use Doctrine\DBAL\Platforms\OraclePlatform;
 use Doctrine\DBAL\Platforms\MySqlPlatform;
+use Doctrine\DBAL\Platforms\SqlitePlatform;
+use Doctrine\DBAL\Platforms\MySQL80Platform;
+use Doctrine\DBAL\Platforms\MariaDb1027Platform;
 use OCP\Files\Folder;
 use OCP\IDBConnection;
 use OCP\DB\QueryBuilder\IQueryBuilder;
@@ -101,13 +104,32 @@ public function process(bool $all = false, int $chunkSize = 1000, \Closure $prog
 	private function queryPreviewsToDelete(int $startFileId = 0, int $chunkSize = 1000): array {
 		$dbPlatform = $this->connection->getDatabasePlatform();
 		$isOracle = ($dbPlatform instanceof OraclePlatform);
+		$isOldMysql = ($dbPlatform instanceof MySqlPlatform && !($dbPlatform instanceof MySQL80Platform || $dbPlatform instanceof MariaDb1027Platform));
 
-		$castTo = 'BIGINT';
+		$castToInt = 'BIGINT';
+		$castToVchar = 'VARCHAR(250)';
 		if ($dbPlatform instanceof MySqlPlatform) {
 			// for MySQL we need to cast to "signed" instead
-			$castTo = 'SIGNED';
+			$castToInt = 'SIGNED';
+			$castToVchar = 'CHAR';
 		} elseif ($isOracle) {
-			$castTo = 'NUMBER';
+			$castToInt = 'NUMBER';
+			$castToVchar = 'VARCHAR2(250)';
+		}
+
+		// The cast condition will be applied to all DB except sqlite (which doesn't
+		// have the regexp_replace function as built-in and requires extensions).
+		// If the name contains only digits, the name will be casted to an integer;
+		// otherwise, the name will be replaced with the fileid. This is intended
+		// because we want to exclude those rows from the final result.
+		$castCondition = "CAST(REGEXP_REPLACE(`thumb`.`name`, '(^.*[^[:digit:]]+.*$|^$)', CAST(`thumb`.`fileid` AS {$castToVchar})) AS {$castToInt})";
+		if ($dbPlatform instanceof SqlitePlatform || $isOldMysql) {
+			// For sqlite, the cast function seems to return 0 if the value can't be
+			// casted properly. We'll use that instead the regexp.
+			// Note that casting "123explode" to bigint will return the 123 integer,
+			// not the 0 we want. We'll rely on the "parent" condition (in the sql
+			// statement) to deal with these false positives.
+			$castCondition = "COALESCE(NULLIF(CAST(`thumb`.`name` AS {$castToInt}), 0), `thumb`.`fileid`)";
 		}
 
 		// for the path_hash -> 3b8779ba05b8f0aed49650f3ff8beb4b = MD5('thumbnails')
@@ -128,7 +150,7 @@ private function queryPreviewsToDelete(int $startFileId = 0, int $chunkSize = 10
 AND NOT EXISTS (
   SELECT 1
   FROM `*PREFIX*filecache`
-  WHERE `fileid` = CAST(`thumb`.`name` AS ${castTo})
+  WHERE `fileid` = {$castCondition}
 )
 AND `fileid` > ?
 ORDER BY `storage`";
