SQL: Same updateCacheUnreads for all DBs (#5648)

* SQL: Same updateCacheUnreads for all DBs
Use same SQL update request for MySQL / MariaDB than the one we already used for PostgreSQL / SQLite (i.e. using a sub-query).

Testing on a DB of 688MB with 270k entries, 199 feeds, 19 categories, using MySQL 8.1.0.

The new SQL update using a sub-query took in average 0.02s, while the old SQL update using a join took in average 0.05s. SQL cache was properly invalidated between each run. The new SQL request is thus about twice faster.

Another advantage of the SQL update using a sub-query is that it works identically in PostgreSQL, SQLite, MariaDB, MySQL, so we do need different versions anymore.

Contributes to https://github.com/FreshRSS/FreshRSS/issues/5008#issuecomment-1709755370

* Force USE INDEX

* Use same SQL methods also for markReadEntries, markReadCat

app/Models/EntryDAO.php

@@ -319,31 +319,29 @@ SQL;
 	 * Update the unread article cache held on every feed details.
 	 * Depending on the parameters, it updates the cache on one feed, on all
 	 * feeds from one category or on all feeds.
-	 *
-	 * @todo It can use the query builder refactoring to build that query
 	 */
 	protected function updateCacheUnreads(?int $catId = null, ?int $feedId = null): bool {
-		$sql = <<<'SQL'
-UPDATE `_feed` f LEFT OUTER JOIN (
-	SELECT e.id_feed, COUNT(*) AS nbUnreads
-	FROM `_entry` e
-	WHERE e.is_read = 0
-	GROUP BY e.id_feed
-) x ON x.id_feed = f.id
-SET f.`cache_nbUnreads` = COALESCE(x.nbUnreads, 0)
+		// Help MySQL/MariaDB's optimizer with the query plan:
+		$useIndex = $this->pdo->dbType() === 'mysql' ? 'USE INDEX (entry_feed_read_index)' : '';
+
+		$sql = <<<SQL
+UPDATE `_feed`
+SET `cache_nbUnreads`=(
+	SELECT COUNT(*) AS nbUnreads FROM `_entry` e {$useIndex}
+	WHERE e.id_feed=`_feed`.id AND e.is_read=0)
 SQL;
 		$hasWhere = false;
 		$values = [];
 		if ($feedId != null) {
 			$sql .= ' WHERE';
 			$hasWhere = true;
-			$sql .= ' f.id=?';
+			$sql .= ' id=?';
 			$values[] = $feedId;
 		}
 		if ($catId != null) {
 			$sql .= $hasWhere ? ' AND' : ' WHERE';
 			$hasWhere = true;
-			$sql .= ' f.category=?';
+			$sql .= ' category=?';
 			$values[] = $catId;
 		}
 		$stm = $this->pdo->prepare($sql);
@@ -360,11 +358,6 @@ SQL;
 	 * Toggle the read marker on one or more article.
 	 * Then the cache is updated.
 	 *
-	 * @todo change the way the query is build because it seems there is
-	 * unnecessary code in here. For instance, the part with the str_repeat.
-	 * @todo remove code duplication. It seems the code is basically the
-	 * same if it is an array or not.
-	 *
 	 * @param string|array<string> $ids
 	 * @param bool $is_read
 	 * @return int|false affected rows
@@ -431,34 +424,26 @@ SQL;
 	 *
 	 * If $idMax equals 0, a deprecated debug message is logged
 	 *
-	 * @todo refactor this method along with markReadCat and markReadFeed
-	 * since they are all doing the same thing. I think we need to build a
-	 * tool to generate the query instead of having queries all over the
-	 * place. It will be reused also for the filtering making every thing
-	 * separated.
-	 *
 	 * @param string $idMax fail safe article ID
 	 * @return int|false affected rows
 	 */
 	public function markReadEntries(string $idMax = '0', bool $onlyFavorites = false, int $priorityMin = 0,
 		?FreshRSS_BooleanSearch $filters = null, int $state = 0, bool $is_read = true) {
 		FreshRSS_UserDAO::touch();
-		if ($idMax == 0) {
+		if ($idMax == '0') {
 			$idMax = time() . '000000';
 			Minz_Log::debug('Calling markReadEntries(0) is deprecated!');
 		}
 
-		$sql = 'UPDATE `_entry` e INNER JOIN `_feed` f ON e.id_feed=f.id '
-			 . 'SET e.is_read=? '
-			 . 'WHERE e.is_read <> ? AND e.id <= ?';
+		$sql = 'UPDATE `_entry` SET is_read = ? WHERE is_read <> ? AND id <= ?';
 		if ($onlyFavorites) {
-			$sql .= ' AND e.is_favorite=1';
+			$sql .= ' AND is_favorite=1';
 		} elseif ($priorityMin >= 0) {
-			$sql .= ' AND f.priority > ' . intval($priorityMin);
+			$sql .= ' AND id_feed IN (SELECT f.id FROM `_feed` f WHERE f.priority > ' . intval($priorityMin) . ')';
 		}
 		$values = [$is_read ? 1 : 0, $is_read ? 1 : 0, $idMax];
 
-		[$searchValues, $search] = $this->sqlListEntriesWhere('e.', $filters, $state);
+		[$searchValues, $search] = $this->sqlListEntriesWhere('', $filters, $state);
 
 		$stm = $this->pdo->prepare($sql . $search);
 		if (!($stm && $stm->execute(array_merge($values, $searchValues)))) {
@@ -491,12 +476,15 @@ SQL;
 			Minz_Log::debug('Calling markReadCat(0) is deprecated!');
 		}
 
-		$sql = 'UPDATE `_entry` e INNER JOIN `_feed` f ON e.id_feed=f.id '
-			 . 'SET e.is_read=? '
-			 . 'WHERE f.category=? AND e.is_read <> ? AND e.id <= ?';
-		$values = [$is_read ? 1 : 0, $id, $is_read ? 1 : 0, $idMax];
+		$sql = <<<'SQL'
+UPDATE `_entry`
+SET is_read = ?
+WHERE is_read <> ? AND id <= ?
+AND id_feed IN (SELECT f.id FROM `_feed` f WHERE f.category=?)
+SQL;
+		$values = [$is_read ? 1 : 0, $is_read ? 1 : 0, $idMax, $id];
 
-		[$searchValues, $search] = $this->sqlListEntriesWhere('e.', $filters, $state);
+		[$searchValues, $search] = $this->sqlListEntriesWhere('', $filters, $state);
 
 		$stm = $this->pdo->prepare($sql . $search);
 		if (!($stm && $stm->execute(array_merge($values, $searchValues)))) {

app/Models/EntryDAOSQLite.php

@@ -65,46 +65,10 @@ SQL;
 		return $result;
 	}
 
-	protected function updateCacheUnreads(?int $catId = null, ?int $feedId = null): bool {
-		$sql = <<<'SQL'
-UPDATE `_feed`
-SET `cache_nbUnreads`=(
-	SELECT COUNT(*) AS nbUnreads FROM `_entry` e
-	WHERE e.id_feed=`_feed`.id AND e.is_read=0)
-SQL;
-		$hasWhere = false;
-		$values = [];
-		if ($feedId != null) {
-			$sql .= ' WHERE';
-			$hasWhere = true;
-			$sql .= ' id=?';
-			$values[] = $feedId;
-		}
-		if ($catId != null) {
-			$sql .= $hasWhere ? ' AND' : ' WHERE';
-			$hasWhere = true;
-			$sql .= ' category=?';
-			$values[] = $catId;
-		}
-		$stm = $this->pdo->prepare($sql);
-		if ($stm !== false && $stm->execute($values)) {
-			return true;
-		} else {
-			$info = $stm == null ? $this->pdo->errorInfo() : $stm->errorInfo();
-			Minz_Log::error('SQL error ' . __METHOD__ . json_encode($info));
-			return false;
-		}
-	}
-
 	/**
 	 * Toggle the read marker on one or more article.
 	 * Then the cache is updated.
 	 *
-	 * @todo change the way the query is build because it seems there is
-	 * unnecessary code in here. For instance, the part with the str_repeat.
-	 * @todo remove code duplication. It seems the code is basically the
-	 * same if it is an array or not.
-	 *
 	 * @param string|array<string> $ids
 	 * @param bool $is_read
 	 * @return int|false affected rows
@@ -148,97 +112,6 @@ SQL;
 		}
 	}
 
-	/**
-	 * Mark all entries as read depending on parameters.
-	 * If $onlyFavorites is true, it is used when the user mark as read in
-	 * the favorite pseudo-category.
-	 * If $priorityMin is greater than 0, it is used when the user mark as
-	 * read in the main feed pseudo-category.
-	 * Then the cache is updated.
-	 *
-	 * If $idMax equals 0, a deprecated debug message is logged
-	 *
-	 * @todo refactor this method along with markReadCat and markReadFeed
-	 * since they are all doing the same thing. I think we need to build a
-	 * tool to generate the query instead of having queries all over the
-	 * place. It will be reused also for the filtering making every thing
-	 * separated.
-	 *
-	 * @param string $idMax fail safe article ID
-	 * @param bool $onlyFavorites
-	 * @param int $priorityMin
-	 * @return int|false affected rows
-	 */
-	public function markReadEntries(string $idMax = '0', bool $onlyFavorites = false, int $priorityMin = 0,
-		?FreshRSS_BooleanSearch $filters = null, int $state = 0, bool $is_read = true) {
-		FreshRSS_UserDAO::touch();
-		if ($idMax == '0') {
-			$idMax = time() . '000000';
-			Minz_Log::debug('Calling markReadEntries(0) is deprecated!');
-		}
-
-		$sql = 'UPDATE `_entry` SET is_read = ? WHERE is_read <> ? AND id <= ?';
-		if ($onlyFavorites) {
-			$sql .= ' AND is_favorite=1';
-		} elseif ($priorityMin >= 0) {
-			$sql .= ' AND id_feed IN (SELECT f.id FROM `_feed` f WHERE f.priority > ' . intval($priorityMin) . ')';
-		}
-		$values = [$is_read ? 1 : 0, $is_read ? 1 : 0, $idMax];
-
-		[$searchValues, $search] = $this->sqlListEntriesWhere('', $filters, $state);
-
-		$stm = $this->pdo->prepare($sql . $search);
-		if (!($stm && $stm->execute(array_merge($values, $searchValues)))) {
-			$info = $stm == null ? $this->pdo->errorInfo() : $stm->errorInfo();
-			Minz_Log::error('SQL error ' . __METHOD__ . json_encode($info));
-			return false;
-		}
-		$affected = $stm->rowCount();
-		if (($affected > 0) && (!$this->updateCacheUnreads(null, null))) {
-			return false;
-		}
-		return $affected;
-	}
-
-	/**
-	 * Mark all the articles in a category as read.
-	 * There is a fail safe to prevent to mark as read articles that are
-	 * loaded during the mark as read action. Then the cache is updated.
-	 *
-	 * If $idMax equals 0, a deprecated debug message is logged
-	 *
-	 * @param int $id category ID
-	 * @param string $idMax fail safe article ID
-	 * @return int|false affected rows
-	 */
-	public function markReadCat(int $id, string $idMax = '0', ?FreshRSS_BooleanSearch $filters = null, int $state = 0, bool $is_read = true) {
-		FreshRSS_UserDAO::touch();
-		if ($idMax == '0') {
-			$idMax = time() . '000000';
-			Minz_Log::debug('Calling markReadCat(0) is deprecated!');
-		}
-
-		$sql = 'UPDATE `_entry` '
-			 . 'SET is_read = ? '
-			 . 'WHERE is_read <> ? AND id <= ? AND '
-			 . 'id_feed IN (SELECT f.id FROM `_feed` f WHERE f.category=?)';
-		$values = [$is_read ? 1 : 0, $is_read ? 1 : 0, $idMax, $id];
-
-		[$searchValues, $search] = $this->sqlListEntriesWhere('', $filters, $state);
-
-		$stm = $this->pdo->prepare($sql . $search);
-		if (!($stm && $stm->execute(array_merge($values, $searchValues)))) {
-			$info = $stm == null ? $this->pdo->errorInfo() : $stm->errorInfo();
-			Minz_Log::error('SQL error ' . __METHOD__ . json_encode($info));
-			return false;
-		}
-		$affected = $stm->rowCount();
-		if (($affected > 0) && (!$this->updateCacheUnreads($id, null))) {
-			return false;
-		}
-		return $affected;
-	}
-
 	/**
 	 * Mark all the articles in a tag as read.
 	 * @param int $id tag ID, or empty for targeting any tag