Merge pull request #9997 from jonasraoni/feature-main-8700-improve-sl…

…ow-query Feature main 8700 improve slow query
pkp · May 30, 2024 · d2ed0ef · d2ed0ef
2 parents 552cac9 + 692b616
commit d2ed0ef
Show file tree

Hide file tree

Showing 40 changed files with 159 additions and 203 deletions.
diff --git a/api/v1/_submissions/PKPBackendSubmissionsController.php b/api/v1/_submissions/PKPBackendSubmissionsController.php
@@ -224,7 +224,7 @@ public function getMany(Request $illuminateRequest): JsonResponse
         $genres = $genreDao->getByContextId($context->getId())->toArray();
 
         return response()->json([
-            'itemsMax' => $collector->limit(null)->offset(null)->getCount(),
+            'itemsMax' => $collector->getCount(),
             'items' => Repo::submission()->getSchemaMap()->mapManyToSubmissionsList($submissions, $userGroups, $genres)->values(),
         ], Response::HTTP_OK);
     }
@@ -259,7 +259,7 @@ public function assigned(Request $illuminateRequest): JsonResponse
         $genres = $genreDao->getByContextId($context->getId())->toArray();
 
         return response()->json([
-            'itemsMax' => $collector->limit(null)->offset(null)->getCount(),
+            'itemsMax' => $collector->getCount(),
             'items' => Repo::submission()->getSchemaMap()->mapManyToSubmissionsList($submissions, $userGroups, $genres)->values(),
         ], Response::HTTP_OK);
     }
@@ -326,7 +326,7 @@ public function reviews(Request $illuminateRequest): JsonResponse
         $genres = $genreDao->getByContextId($context->getId())->toArray();
 
         return response()->json([
-            'itemsMax' => $collector->limit(null)->offset(null)->getCount(),
+            'itemsMax' => $collector->getCount(),
             'items' => Repo::submission()->getSchemaMap()->mapManyToSubmissionsList($submissions, $userGroups, $genres)->values(),
         ], Response::HTTP_OK);
     }
@@ -384,7 +384,7 @@ public function getReviewAssignments(Request $illuminateRequest): JsonResponse
         $reviewAssignments = $collector->getMany();
 
         return response()->json([
-            'itemsMax' => $collector->limit(null)->offset(null)->getCount(),
+            'itemsMax' => $collector->getCount(),
             'items' => Repo::reviewAssignment()->getSchemaMap()->mapMany($reviewAssignments)->values(),
         ], Response::HTTP_OK);
     }

diff --git a/api/v1/announcements/PKPAnnouncementController.php b/api/v1/announcements/PKPAnnouncementController.php
@@ -184,7 +184,7 @@ public function getMany(Request $illuminateRequest): JsonResponse
         $announcements = $collector->getMany();
 
         return response()->json([
-            'itemsMax' => $collector->limit(null)->offset(null)->getCount(),
+            'itemsMax' => $collector->getCount(),
             'items' => Repo::announcement()->getSchemaMap()->summarizeMany($announcements)->values(),
         ], Response::HTTP_OK);
     }

diff --git a/api/v1/dois/PKPDoiController.php b/api/v1/dois/PKPDoiController.php
@@ -204,7 +204,7 @@ public function getMany(Request $illuminateRequest): JsonResponse
         $dois = $collector->getMany();
 
         return response()->json([
-            'itemsMax' => $collector->limit(null)->offset(0)->getCount(),
+            'itemsMax' => $collector->getCount(),
             'items' => Repo::doi()->getSchemaMap()->summarizeMany($dois)->values(),
         ], Response::HTTP_OK);
     }

diff --git a/api/v1/emailTemplates/PKPEmailTemplateController.php b/api/v1/emailTemplates/PKPEmailTemplateController.php
@@ -154,7 +154,7 @@ public function getMany(Request $illuminateRequest): JsonResponse
         $emailTemplates = $collector->getMany();
 
         return response()->json([
-            'itemsMax' => $collector->limit(null)->offset(null)->getCount(),
+            'itemsMax' => $collector->getCount(),
             'items' => Repo::emailTemplate()->getSchemaMap()->summarizeMany($emailTemplates),
         ], Response::HTTP_OK);
     }

diff --git a/api/v1/highlights/HighlightsController.php b/api/v1/highlights/HighlightsController.php
@@ -150,7 +150,7 @@ public function getMany(Request $illuminateRequest): JsonResponse
         $highlights = $collector->getMany();
 
         return response()->json([
-            'itemsMax' => $collector->limit(null)->offset(null)->getCount(),
+            'itemsMax' => $collector->getCount(),
             'items' => Repo::highlight()->getSchemaMap()->summarizeMany($highlights)->values(),
         ], Response::HTTP_OK);
     }

diff --git a/api/v1/institutions/PKPInstitutionController.php b/api/v1/institutions/PKPInstitutionController.php
@@ -154,7 +154,7 @@ public function getMany(Request $illuminateRequest): JsonResponse
         $institutions = $collector->getMany();
 
         return response()->json([
-            'itemsMax' => $collector->limit(null)->offset(null)->getCount(),
+            'itemsMax' => $collector->getCount(),
             'items' => Repo::institution()->getSchemaMap()->summarizeMany($institutions->values())->values(),
         ], Response::HTTP_OK);
     }

diff --git a/api/v1/sections/SectionController.php b/api/v1/sections/SectionController.php
@@ -151,7 +151,7 @@ public function getMany(Request $illuminateRequest): JsonResponse
         $sections = $collector->getMany();
 
         return response()->json([
-            'itemsMax' => $collector->limit(null)->offset(null)->getCount(),
+            'itemsMax' => $collector->getCount(),
             'items' => Repo::section()->getSchemaMap()->summarizeMany($sections)->values(),
         ], Response::HTTP_OK);
     }

diff --git a/api/v1/submissions/PKPSubmissionController.php b/api/v1/submissions/PKPSubmissionController.php
@@ -362,7 +362,7 @@ public function getMany(Request $illuminateRequest): JsonResponse
         $genres = $genreDao->getByContextId($context->getId())->toArray();
 
         return response()->json([
-            'itemsMax' => $collector->limit(null)->offset(null)->getCount(),
+            'itemsMax' => $collector->getCount(),
             'items' => Repo::submission()->getSchemaMap()->summarizeMany($submissions, $userGroups, $genres)->values(),
         ], Response::HTTP_OK);
     }
@@ -900,7 +900,7 @@ public function getPublications(Request $illuminateRequest): JsonResponse
         $genres = $genreDao->getByContextId($submission->getData('contextId'))->toArray();
 
         return response()->json([
-            'itemsMax' => $collector->limit(null)->offset(null)->getCount(),
+            'itemsMax' => $collector->getCount(),
             'items' => Repo::publication()->getSchemaMap($submission, $userGroups, $genres)->summarizeMany($publications, $anonymize)->values(),
         ], Response::HTTP_OK);
     }
@@ -1357,7 +1357,7 @@ public function getContributors(Request $illuminateRequest): JsonResponse
         $authors = $collector->getMany();
 
         return response()->json([
-            'itemsMax' => $collector->limit(null)->offset(null)->getCount(),
+            'itemsMax' => $collector->getCount(),
             'items' => Repo::author()->getSchemaMap()->summarizeMany($authors)->values(),
         ], Response::HTTP_OK);
     }

diff --git a/api/v1/users/PKPUserController.php b/api/v1/users/PKPUserController.php
@@ -176,7 +176,7 @@ public function getMany(Request $request): JsonResponse
         }
 
         return response()->json([
-            'itemsMax' => $collector->limit(null)->offset(null)->getCount(),
+            'itemsMax' => $collector->getCount(),
             'items' => $items,
         ], Response::HTTP_OK);
     }
@@ -230,7 +230,7 @@ public function getReviewers(Request $request): JsonResponse
         }
 
         return response()->json([
-            'itemsMax' => $collector->limit(null)->offset(null)->getCount(),
+            'itemsMax' => $collector->getCount(),
             'items' => $items,
         ], Response::HTTP_OK);
     }

diff --git a/classes/announcement/DAO.php b/classes/announcement/DAO.php
@@ -83,8 +83,7 @@ public function getCount(Collector $query): int
     {
         return $query
             ->getQueryBuilder()
-            ->get('a.' . $this->primaryKeyColumn)
-            ->count();
+            ->getCountForPagination();
     }
 
     /**

diff --git a/classes/author/DAO.php b/classes/author/DAO.php
@@ -113,7 +113,7 @@ public function getCount(Collector $query): int
     {
         return $query
             ->getQueryBuilder()
-            ->count();
+            ->getCountForPagination();
     }
 
     /**

diff --git a/classes/category/DAO.php b/classes/category/DAO.php
@@ -71,7 +71,7 @@ public function newDataObject(): Category
      */
     public function getCount(Collector $query): int
     {
-        return $query->getQueryBuilder()->count();
+        return $query->getQueryBuilder()->getCountForPagination();
     }
 
     /**

diff --git a/classes/components/listPanels/PKPSelectReviewerListPanel.php b/classes/components/listPanels/PKPSelectReviewerListPanel.php
@@ -186,7 +186,7 @@ public function getItems($request)
      */
     public function getItemsMax()
     {
-        return $this->_getCollector()->offset(null)->limit(null)->getCount();
+        return $this->_getCollector()->getCount();
     }
 
     /**

diff --git a/classes/db/DAO.php b/classes/db/DAO.php
@@ -24,6 +24,7 @@
 namespace PKP\db;
 
 use Generator;
+use Illuminate\Database\Query\Builder;
 use Illuminate\Support\Facades\DB;
 use PKP\cache\CacheManager;
 use PKP\core\JSONMessage;
@@ -80,8 +81,8 @@ public function retrieve($sql, $params = [], $callHooks = true)
     /**
      * Execute a SELECT SQL statement, returning rows in the range supplied.
      *
-     * @param string $sql the SQL statement
-     * @param array $params parameters for the SQL statement
+     * @param string|Builder $sql the SQL statement
+     * @param array $params parameters for the SQL statement, params is used only when $sql is a string
      * @param DBResultRange $dbResultRange object describing the desired range
      *
      * @deprecated 3.4
@@ -102,27 +103,35 @@ public function retrieveRange($sql, $params = [], $dbResultRange = null, $callHo
         }
 
         if ($dbResultRange && $dbResultRange->isValid()) {
-            $sql .= ' LIMIT ' . (int) $dbResultRange->getCount();
+            $limit = (int) $dbResultRange->getCount();
             $offset = (int) $dbResultRange->getOffset();
             $offset += max(0, $dbResultRange->getPage() - 1) * (int) $dbResultRange->getCount();
-            $sql .= ' OFFSET ' . $offset;
+            if ($sql instanceof Builder) {
+                $sql->limit($limit)->offset($offset);
+            } else {
+                $sql .= " LIMIT {$limit} OFFSET {$offset}";
+            }
         }
 
-        return DB::cursor(DB::raw($sql)->getValue(DB::connection()->getQueryGrammar()), $params);
+        return $sql instanceof Builder ? $sql->get() : DB::cursor(DB::raw($sql)->getValue(DB::connection()->getQueryGrammar()), $params);
     }
 
     /**
      * Count the number of records in the supplied SQL statement (with optional bind parameters parameters)
      *
-     * @param string $sql SQL query to be counted
-     * @param array $params Optional SQL query bind parameters
+     * @param string|Builder $sql SQL query to be counted
+     * @param array $params Optional SQL query bind parameters, only used when the $sql argument is a string
      *
      * @deprecated 3.4
      *
      * @return int
      */
     public function countRecords($sql, $params = [])
     {
+        // In case a Laravel Builder has been received, drop its SELECT and ORDER BY clauses for optimization purposes
+        if ($sql instanceof Builder) {
+            return $sql->getCountForPagination();
+        }
         $result = $this->retrieve('SELECT COUNT(*) AS row_count FROM (' . $sql . ') AS count_subquery', $params);
         return $result->current()->row_count;
     }

diff --git a/classes/db/DAOResultFactory.php b/classes/db/DAOResultFactory.php
@@ -18,6 +18,8 @@
 namespace PKP\db;
 
 use APP\submission\DAO;
+use Countable;
+use Illuminate\Database\Query\Builder;
 use Illuminate\Support\Collection;
 use Illuminate\Support\Enumerable;
 use PKP\core\ItemIterator;
@@ -46,7 +48,7 @@ class DAOResultFactory extends ItemIterator
     public $records;
 
     /**
-     * @var string|null Fetch SQL
+     * @var string|Builder|null Fetch SQL
      */
     public $sql;
 
@@ -76,8 +78,8 @@ class DAOResultFactory extends ItemIterator
      * @param object $dao DAO class for factory
      * @param string $functionName The function to call on $dao to create an object
      * @param array $idFields an array of primary key field names that uniquely identify a result row in the record set. Should be data object _data array key, not database column name
-     * @param string $sql Optional SQL query used to generate paged result set. Necessary when total row counts will be needed (e.g. when paging). WARNING: New code should not use this.
-     * @param array $params Optional parameters for SQL query used to generate paged result set. Necessary when total row counts will be needed (e.g. when paging). WARNING: New code should not use this.
+     * @param string|Builder|null $sql Optional SQL query used to generate paged result set. Necessary when total row counts will be needed (e.g. when paging). WARNING: New code should not use this.
+     * @param array $params Optional parameters for SQL query used to generate paged result set. Necessary when total row counts will be needed (e.g. when paging), only used when the $sql argument is a string. WARNING: New code should not use this.
      * @param ?DBResultRange $rangeInfo Optional pagination information. WARNING: New code should not use this.
      */
     public function __construct($records, $dao, $functionName, $idFields = [], $sql = null, $params = [], $rangeInfo = null)
@@ -207,9 +209,8 @@ public function eof()
         if ($this->records == null) {
             return true;
         }
-        /** @var DAOResultIterator */
-        $records = $this->records;
-        return !$records->valid();
+
+        return $this->records instanceof Countable ? !count($this->records) : !$this->records->valid();
     }
 
     /**

diff --git a/classes/decision/DAO.php b/classes/decision/DAO.php
@@ -77,7 +77,7 @@ public function getCount(Collector $query): int
     {
         return $query
             ->getQueryBuilder()
-            ->count();
+            ->getCountForPagination();
     }
 
     /**

diff --git a/classes/doi/DAO.php b/classes/doi/DAO.php
@@ -79,9 +79,7 @@ public function getCount(Collector $query): int
     {
         return $query
             ->getQueryBuilder()
-            ->select('d.' . $this->primaryKeyColumn)
-            ->get()
-            ->count();
+            ->getCountForPagination();
     }
 
     /**

diff --git a/classes/doi/Repository.php b/classes/doi/Repository.php
@@ -326,8 +326,8 @@ public function isAssigned(int $doiId, string $pubObjectType): bool
             Repo::doi()::TYPE_PUBLICATION => Repo::publication()
                 ->getCollector()
                 ->filterByDoiIds([$doiId])
-                ->getIds()
-                ->count(),
+                ->getQueryBuilder()
+                ->getCountForPagination() > 0,
             default => false,
         };
     }

diff --git a/classes/emailTemplate/DAO.php b/classes/emailTemplate/DAO.php
@@ -147,8 +147,7 @@ public function getCount(Collector $query): int
     {
         return $query
             ->getQueryBuilder()
-            ->get()
-            ->count();
+            ->getCountForPagination();
     }
 
     /**
-Original file line number
+Diff line change
@@ Expand Up / @@ -71,7 +71,7 @@ public function newDataObject(): Category @@
          */
         public function getCount(Collector $query): int
         {
-            return $query->getQueryBuilder()->count();
+            return $query->getQueryBuilder()->getCountForPagination();
         }
         /**
@@ Expand Down @@