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

1 files changed, 23 insertions, 35 deletions

diff --git a/app/Models/EntryDAO.php b/app/Models/EntryDAO.php
index 8a006f802..f57fa17d7 100644
--- a/app/Models/EntryDAO.php
+++ b/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)))) {