From 759076c4ac3b118417c76bf63de1df8ea1b60a94 Mon Sep 17 00:00:00 2001 From: Varun Patil Date: Thu, 27 Oct 2022 09:19:25 -0700 Subject: [PATCH] Use INNER JOIN for CTE --- lib/Controller/ApiController.php | 2 +- lib/Db/TimelineQuery.php | 15 ++++ lib/Db/TimelineQueryAlbums.php | 2 +- lib/Db/TimelineQueryDays.php | 118 +++++++++++++++---------------- lib/Db/TimelineQueryFaces.php | 10 +-- lib/Db/TimelineQueryTags.php | 15 ++-- 6 files changed, 87 insertions(+), 75 deletions(-) diff --git a/lib/Controller/ApiController.php b/lib/Controller/ApiController.php index 5e40965f..205448be 100644 --- a/lib/Controller/ApiController.php +++ b/lib/Controller/ApiController.php @@ -278,7 +278,7 @@ class ApiController extends Controller // Run actual query $list = []; - $t = intval($this->request->getParam('t')); + $t = (int) ($this->request->getParam('t')); if ($t & 1) { // personal $list = array_merge($list, $this->timelineQuery->getAlbums($user->getUID())); } diff --git a/lib/Db/TimelineQuery.php b/lib/Db/TimelineQuery.php index 53d22095..def3035f 100644 --- a/lib/Db/TimelineQuery.php +++ b/lib/Db/TimelineQuery.php @@ -4,6 +4,7 @@ declare(strict_types=1); namespace OCA\Memories\Db; +use OCP\DB\QueryBuilder\IQueryBuilder; use OCP\IDBConnection; class TimelineQuery @@ -21,6 +22,20 @@ class TimelineQuery $this->connection = $connection; } + public static function debugQuery(IQueryBuilder $query, string $sql = '') + { + // Print the query and exit + $sql = empty($sql) ? $query->getSQL() : $sql; + $params = $query->getParameters(); + $sql = str_replace('*PREFIX*', 'oc_', $sql); + foreach ($params as $key => $value) { + $sql = str_replace(':'.$key, $query->getConnection()->getDatabasePlatform()->quoteStringLiteral($value), $sql); + } + echo "{$sql}"; + + exit; + } + public function getInfoById(int $id): array { $qb = $this->connection->getQueryBuilder(); diff --git a/lib/Db/TimelineQueryAlbums.php b/lib/Db/TimelineQueryAlbums.php index 6f391adc..55bc4c4a 100644 --- a/lib/Db/TimelineQueryAlbums.php +++ b/lib/Db/TimelineQueryAlbums.php @@ -30,7 +30,7 @@ trait TimelineQueryAlbums } /** Get list of albums */ - public function getAlbums(string $uid, $shared=false) + public function getAlbums(string $uid, $shared = false) { $query = $this->connection->getQueryBuilder(); diff --git a/lib/Db/TimelineQueryDays.php b/lib/Db/TimelineQueryDays.php index ce04ac33..5d0382bf 100644 --- a/lib/Db/TimelineQueryDays.php +++ b/lib/Db/TimelineQueryDays.php @@ -8,6 +8,26 @@ use OCP\DB\QueryBuilder\IQueryBuilder; use OCP\Files\Folder; use OCP\IDBConnection; +const CTE_FOLDERS = // CTE to get all folders recursively in the given top folder + 'WITH RECURSIVE *PREFIX*cte_folders(fileid) AS ( + SELECT + f.fileid + FROM + *PREFIX*filecache f + WHERE + f.fileid = :topFolderId + UNION ALL + SELECT + f.fileid + FROM + *PREFIX*filecache f + INNER JOIN *PREFIX*cte_folders c + ON (f.parent = c.fileid + AND f.mimetype = 2 + AND f.fileid <> :excludedFolderId + ) + )'; + trait TimelineQueryDays { protected IDBConnection $connection; @@ -35,8 +55,8 @@ trait TimelineQueryDays $count = $query->func()->count($query->createFunction('DISTINCT m.fileid'), 'count'); $query->select('m.dayid', $count) ->from('memories', 'm') - ->innerJoin('m', 'filecache', 'f', $this->getFilecacheJoinQuery($query, $folder, $recursive, $archive)) ; + $query = $this->joinFilecache($query, $folder, $recursive, $archive); // Group and sort by dayid $query->groupBy('m.dayid') @@ -46,7 +66,7 @@ trait TimelineQueryDays // Apply all transformations $this->applyAllTransforms($queryTransforms, $query, $uid); - $cursor = $query->executeQuery(); + $cursor = $this->executeQueryWithCTEs($query); $rows = $cursor->fetchAll(); $cursor->closeCursor(); @@ -84,8 +104,8 @@ trait TimelineQueryDays // when using DISTINCT on selected fields $query->select($fileid, 'f.etag', 'm.isvideo', 'vco.categoryid', 'm.datetaken', 'm.dayid', 'm.w', 'm.h') ->from('memories', 'm') - ->innerJoin('m', 'filecache', 'f', $this->getFilecacheJoinQuery($query, $folder, $recursive, $archive)) ; + $query = $this->joinFilecache($query, $folder, $recursive, $archive); // Filter by dayid unless wildcard if (null !== $day_ids) { @@ -105,7 +125,7 @@ trait TimelineQueryDays // Apply all transformations $this->applyAllTransforms($queryTransforms, $query, $uid); - $cursor = $query->executeQuery(); + $cursor = $this->executeQueryWithCTEs($query); $rows = $cursor->fetchAll(); $cursor->closeCursor(); @@ -159,43 +179,31 @@ trait TimelineQueryDays return $day; } + private function executeQueryWithCTEs(IQueryBuilder &$query) + { + $sql = $query->getSQL(); + $params = $query->getParameters(); + $types = $query->getParameterTypes(); + + // Add WITH clause if needed + if (false !== strpos($sql, 'cte_folders')) { + $sql = CTE_FOLDERS.' '.$sql; + } + + return $this->connection->executeQuery($sql, $params, $types); + } + /** * Get all folders inside a top folder. */ - private function getSubfolderIdsRecursive( - IDBConnection &$conn, + private function joinSubfoldersRecursive( + IQueryBuilder &$query, Folder &$folder, bool $archive ) { - // CTE to get all folders recursively in the given top folder - $cte = - 'WITH RECURSIVE cte_folders(fileid) AS ( - SELECT - f.fileid - FROM - *PREFIX*filecache f - WHERE - f.fileid = :topFolderId - UNION ALL - SELECT - f.fileid - FROM - *PREFIX*filecache f - INNER JOIN cte_folders c - ON (f.parent = c.fileid - AND f.mimetype = 2 - AND f.fileid NOT IN (:excludedFolderIds) - ) - ) - SELECT - fileid - FROM - cte_folders - '; - // Query parameters, set at the end $topFolderId = $folder->getId(); - $excludedFolderIds = [-1]; // cannot be empty + $excludedFolderId = -1; /** @var Folder Archive folder if it exists */ $archiveFolder = null; @@ -208,31 +216,28 @@ trait TimelineQueryDays if (!$archive) { // Exclude archive folder if ($archiveFolder) { - $excludedFolderIds[] = $archiveFolder->getId(); + $excludedFolderId = $archiveFolder->getId(); } } else { // Only include archive folder $topFolderId = $archiveFolder ? $archiveFolder->getId() : -1; } - return array_map('intval', array_column($conn->executeQuery($cte, [ - 'topFolderId' => $topFolderId, - 'excludedFolderIds' => $excludedFolderIds, - ], [ - 'topFolderId' => IQueryBuilder::PARAM_INT, - 'excludedFolderIds' => IQueryBuilder::PARAM_INT_ARRAY, - ])->fetchAll(), 'fileid')); + // Add query parameters + $query->setParameter('topFolderId', $topFolderId, IQueryBuilder::PARAM_INT); + $query->setParameter('excludedFolderId', $excludedFolderId, IQueryBuilder::PARAM_INT); + $query->innerJoin('f', 'cte_folders', 'cte_f', $query->expr()->eq('f.parent', 'cte_f.fileid')); } /** - * Get the query for oc_filecache join. + * Inner join with oc_filecache. * - * @param IQueryBuilder $query Query builder - * @param null|array|Folder $folder Either the top folder or array of folder Ids - * @param bool $recursive Whether to get the days recursively - * @param bool $archive Whether to get the days only from the archive folder + * @param IQueryBuilder $query Query builder + * @param null|Folder $folder Either the top folder or null for all + * @param bool $recursive Whether to get the days recursively + * @param bool $archive Whether to get the days only from the archive folder */ - private function getFilecacheJoinQuery( + private function joinFilecache( IQueryBuilder &$query, &$folder, bool $recursive, @@ -241,31 +246,22 @@ trait TimelineQueryDays // Join with memories $baseOp = $query->expr()->eq('f.fileid', 'm.fileid'); if (null === $folder) { - return $baseOp; // No folder, get all + return $query->innerJoin('m', 'filecache', 'f', $baseOp); } // Filter by folder (recursive or otherwise) $pathOp = null; if ($recursive) { - // Get all subfolder Ids recursively - $folderIds = []; - if ($folder instanceof Folder) { - $conn = $query->getConnection(); - $folderIds = $this->getSubfolderIdsRecursive($conn, $folder, $archive); - } else { - $folderIds = $folder; - } - - // Join with folder IDs - $pathOp = $query->expr()->in('f.parent', $query->createNamedParameter($folderIds, IQueryBuilder::PARAM_INT_ARRAY)); + // Join with folders CTE + $this->joinSubfoldersRecursive($query, $folder, $archive); } else { // If getting non-recursively folder only check for parent $pathOp = $query->expr()->eq('f.parent', $query->createNamedParameter($folder->getId(), IQueryBuilder::PARAM_INT)); } - return $query->expr()->andX( + return $query->innerJoin('m', 'filecache', 'f', $query->expr()->andX( $baseOp, $pathOp, - ); + )); } } diff --git a/lib/Db/TimelineQueryFaces.php b/lib/Db/TimelineQueryFaces.php index 33f005e2..ed01fd2e 100644 --- a/lib/Db/TimelineQueryFaces.php +++ b/lib/Db/TimelineQueryFaces.php @@ -62,7 +62,7 @@ trait TimelineQueryFaces $query->innerJoin('rfd', 'memories', 'm', $query->expr()->eq('m.fileid', 'rfd.file_id')); // WHERE these photos are in the user's requested folder recursively - $query->innerJoin('m', 'filecache', 'f', $this->getFilecacheJoinQuery($query, $folder, true, false)); + $query = $this->joinFilecache($query, $folder, true, false); // GROUP by ID of face cluster $query->groupBy('rfc.id'); @@ -73,7 +73,8 @@ trait TimelineQueryFaces $query->addOrderBy('rfc.id'); // tie-breaker // FETCH all faces - $faces = $query->executeQuery()->fetchAll(); + $cursor = $this->executeQueryWithCTEs($query); + $faces = $cursor->fetchAll(); // Post process foreach ($faces as &$row) { @@ -108,7 +109,7 @@ trait TimelineQueryFaces $query->innerJoin('rfd', 'memories', 'm', $query->expr()->eq('m.fileid', 'rfd.file_id')); // WHERE these photos are in the user's requested folder recursively - $query->innerJoin('m', 'filecache', 'f', $this->getFilecacheJoinQuery($query, $folder, true, false)); + $query = $this->joinFilecache($query, $folder, true, false); // LIMIT results $query->setMaxResults(15); @@ -118,7 +119,8 @@ trait TimelineQueryFaces $query->addOrderBy('m.fileid', 'DESC'); // tie-breaker // FETCH face detections - $previews = $query->executeQuery()->fetchAll(); + $cursor = $this->executeQueryWithCTEs($query); + $previews = $cursor->fetchAll(); if (empty($previews)) { return null; } diff --git a/lib/Db/TimelineQueryTags.php b/lib/Db/TimelineQueryTags.php index 80af1d8c..3af7079f 100644 --- a/lib/Db/TimelineQueryTags.php +++ b/lib/Db/TimelineQueryTags.php @@ -50,15 +50,15 @@ trait TimelineQueryTags // WHERE there are items with this tag $query->innerJoin('st', 'systemtag_object_mapping', 'stom', $query->expr()->andX( - $query->expr()->eq('stom.systemtagid', 'st.id'), $query->expr()->eq('stom.objecttype', $query->createNamedParameter('files')), + $query->expr()->eq('stom.systemtagid', 'st.id'), )); // WHERE these items are memories indexed photos $query->innerJoin('stom', 'memories', 'm', $query->expr()->eq('m.fileid', 'stom.objectid')); // WHERE these photos are in the user's requested folder recursively - $query->innerJoin('m', 'filecache', 'f', $this->getFilecacheJoinQuery($query, $folder, true, false)); + $query = $this->joinFilecache($query, $folder, true, false); // GROUP and ORDER by tag name $query->groupBy('st.name'); @@ -66,7 +66,8 @@ trait TimelineQueryTags $query->addOrderBy('st.id'); // tie-breaker // FETCH all tags - $tags = $query->executeQuery()->fetchAll(); + $cursor = $this->executeQueryWithCTEs($query); + $tags = $cursor->fetchAll(); // Post process foreach ($tags as &$row) { @@ -79,9 +80,6 @@ trait TimelineQueryTags public function getTagPreviews(array &$tags, Folder &$folder) { - // Cache subfolder ids to prevent duplicate requests - $folderIds = $this->getSubfolderIdsRecursive($this->connection, $folder, false); - foreach ($tags as &$tag) { $query = $this->connection->getQueryBuilder(); @@ -98,13 +96,14 @@ trait TimelineQueryTags $query->innerJoin('stom', 'memories', 'm', $query->expr()->eq('m.fileid', 'stom.objectid')); // WHERE these photos are in the user's requested folder recursively - $query->innerJoin('m', 'filecache', 'f', $this->getFilecacheJoinQuery($query, $folderIds, true, false)); + $query = $this->joinFilecache($query, $folder, true, false); // MAX 4 $query->setMaxResults(4); // FETCH tag previews - $tag['previews'] = $query->executeQuery()->fetchAll(); + $cursor = $this->executeQueryWithCTEs($query); + $tag['previews'] = $cursor->fetchAll(); // Post-process foreach ($tag['previews'] as &$row) {