Merge pull request pkp#4286 from jonasraoni/feature/stable-3_4_0/8700…

…-improve-slow-query Feature/stable 3 4 0/8700 improve slow query
touhidurabir · May 30, 2024 · c2eff2e · c2eff2e
2 parents 7859f15 + 04606dc
commit c2eff2e
Show file tree

Hide file tree

Showing 28 changed files with 392 additions and 439 deletions.
diff --git a/api/v1/issues/IssueHandler.php b/api/v1/issues/IssueHandler.php
@@ -204,7 +204,7 @@ public function getMany($slimRequest, $response, $args)
 
         return $response->withJson([
             'items' => Repo::issue()->getSchemaMap()->summarizeMany($issues, $context)->values(),
-            'itemsMax' => $collector->limit(null)->offset(null)->getCount(),
+            'itemsMax' => $collector->getCount(),
         ], 200);
     }
 

diff --git a/classes/author/DAO.php b/classes/author/DAO.php
@@ -111,45 +111,48 @@ public function getAuthorsAlphabetizedByJournal($journalId = null, $initial = nu
             $initialSql .= ')';
         }
 
-        $result = $this->deprecatedDao->retrieveRange(
-            $sql = 'SELECT a.*, ug.show_title, s.locale,
-                COALESCE(agl.setting_value, agpl.setting_value) AS author_given,
-                CASE WHEN agl.setting_value <> \'\' THEN afl.setting_value ELSE afpl.setting_value END AS author_family
-            FROM	authors a
-                JOIN user_groups ug ON (a.user_group_id = ug.user_group_id)
-                JOIN publications p ON (p.publication_id = a.publication_id)
-                JOIN submissions s ON (s.current_publication_id = p.publication_id)
-                LEFT JOIN author_settings agl ON (a.author_id = agl.author_id AND agl.setting_name = ? AND agl.locale = ?)
-                LEFT JOIN author_settings agpl ON (a.author_id = agpl.author_id AND agpl.setting_name = ? AND agpl.locale = s.locale)
-                LEFT JOIN author_settings afl ON (a.author_id = afl.author_id AND afl.setting_name = ? AND afl.locale = ?)
-                LEFT JOIN author_settings afpl ON (a.author_id = afpl.author_id AND afpl.setting_name = ? AND afpl.locale = s.locale)
-                JOIN (
-                    SELECT
+        $baseSql = '
+            FROM authors a
+            JOIN user_groups ug ON (a.user_group_id = ug.user_group_id)
+            JOIN publications p ON (p.publication_id = a.publication_id)
+            JOIN submissions s ON (s.current_publication_id = p.publication_id)
+            LEFT JOIN author_settings agl ON (a.author_id = agl.author_id AND agl.setting_name = ? AND agl.locale = ?)
+            LEFT JOIN author_settings agpl ON (a.author_id = agpl.author_id AND agpl.setting_name = ? AND agpl.locale = s.locale)
+            LEFT JOIN author_settings afl ON (a.author_id = afl.author_id AND afl.setting_name = ? AND afl.locale = ?)
+            LEFT JOIN author_settings afpl ON (a.author_id = afpl.author_id AND afpl.setting_name = ? AND afpl.locale = s.locale)
+            JOIN (
+                SELECT
                     MIN(aa.author_id) as author_id,
                     CONCAT(
-                    ' . ($includeEmail ? 'aa.email, \' \', ' : '') . '
-                    ac.setting_value,
-                    \' \'
-                    ' . $sqlColumnsAuthorSettings . '
+                        ' . ($includeEmail ? "aa.email, ' ', " : '') . "
+                        ac.setting_value,
+                        ' '
+                        {$sqlColumnsAuthorSettings}
                     ) as names
-                    FROM authors aa
-                    JOIN publications pp ON (pp.publication_id = aa.publication_id)
-                    LEFT JOIN publication_settings ppss ON (ppss.publication_id = pp.publication_id)
-                    JOIN submissions ss ON (ss.submission_id = pp.submission_id AND ss.current_publication_id = pp.publication_id AND ss.status = ' . PKPSubmission::STATUS_PUBLISHED . ')
-                    JOIN journals j ON (ss.context_id = j.journal_id)
-                    JOIN issues i ON (ppss.setting_name = ? AND ppss.setting_value = CAST(i.issue_id AS CHAR(20)) AND i.published = 1)
-                    LEFT JOIN author_settings ac ON (ac.author_id = aa.author_id AND ac.setting_name = \'country\')
-                    ' . $sqlJoinAuthorSettings . '
-                    WHERE j.enabled = 1
-                    ' . (isset($journalId) ? ' AND j.journal_id = ?' : '')
-                    . $initialSql . '
-                    GROUP BY names
-                ) as t1 ON (t1.author_id = a.author_id)
-                ORDER BY author_family, author_given',
+                FROM authors aa
+                JOIN publications pp ON (pp.publication_id = aa.publication_id)
+                LEFT JOIN publication_settings ppss ON (ppss.publication_id = pp.publication_id)
+                JOIN submissions ss ON (ss.submission_id = pp.submission_id AND ss.current_publication_id = pp.publication_id AND ss.status = " . PKPSubmission::STATUS_PUBLISHED . ")
+                JOIN journals j ON (ss.context_id = j.journal_id)
+                JOIN issues i ON (ppss.setting_name = ? AND ppss.setting_value = CAST(i.issue_id AS CHAR(20)) AND i.published = 1)
+                LEFT JOIN author_settings ac ON (ac.author_id = aa.author_id AND ac.setting_name = 'country')
+                {$sqlJoinAuthorSettings}
+                WHERE j.enabled = 1
+                " . (isset($journalId) ? ' AND j.journal_id = ?' : '') . "
+                {$initialSql}
+                GROUP BY names
+            ) as t1 ON (t1.author_id = a.author_id)";
+
+        $result = $this->deprecatedDao->retrieveRange(
+            "SELECT a.*, ug.show_title, s.locale,
+            COALESCE(agl.setting_value, agpl.setting_value) AS author_given,
+            CASE WHEN agl.setting_value <> '' THEN afl.setting_value ELSE afpl.setting_value END AS author_family
+            {$baseSql}
+            ORDER BY author_family, author_given",
             $params,
             $rangeInfo
         );
 
-        return new DAOResultFactory($result, $this, 'fromRow', [], $sql, $params, $rangeInfo);
+        return new DAOResultFactory($result, $this, 'fromRow', [], "SELECT 0 {$baseSql}", $params, $rangeInfo);
     }
 }
diff --git a/classes/doi/Repository.php b/classes/doi/Repository.php
@@ -306,8 +306,8 @@ public function isAssigned(int $doiId, string $pubObjectType): bool
             Repo::doi()::TYPE_REPRESENTATION => Repo::galley()
                 ->getCollector()
                 ->filterByDoiIds([$doiId])
-                ->getIds()
-                ->count(),
+                ->getQueryBuilder()
+                ->getCountForPagination() > 0,
             default => false,
         };
 

diff --git a/classes/issue/DAO.php b/classes/issue/DAO.php
@@ -16,6 +16,9 @@
 namespace APP\issue;
 
 use APP\facades\Repo;
+use APP\plugins\PubObjectsExportPlugin;
+use Illuminate\Database\Query\Builder;
+use Illuminate\Database\Query\JoinClause;
 use Illuminate\Support\Collection;
 use Illuminate\Support\Facades\DB;
 use Illuminate\Support\LazyCollection;
@@ -133,7 +136,7 @@ public function getCount(Collector $query): int
     {
         return $query
             ->getQueryBuilder()
-            ->count();
+            ->getCountForPagination();
     }
 
     /**
@@ -266,10 +269,9 @@ public function resequenceCustomIssueOrders(int $contextId)
      */
     public function customIssueOrderingExists(int $contextId): bool
     {
-        $resultCount = DB::table('custom_issue_orders', 'o')
+        return DB::table('custom_issue_orders', 'o')
             ->where('o.journal_id', '=', $contextId)
-            ->count();
-        return $resultCount != 0;
+            ->getCountForPagination() > 0;
     }
 
     /**
@@ -432,38 +434,30 @@ public function deleteAllPubIds($contextId, $pubIdType)
      */
     public function getExportable($contextId, $pubIdType = null, $pubIdSettingName = null, $pubIdSettingValue = null, $rangeInfo = null)
     {
-        $params = [];
-        if ($pubIdSettingName) {
-            $params[] = $pubIdSettingName;
-        }
-        $params[] = (int) $contextId;
-        if ($pubIdType) {
-            $params[] = 'pub-id::' . $pubIdType;
-        }
-
-        import('classes.plugins.PubObjectsExportPlugin'); // Constants
-        if ($pubIdSettingName && $pubIdSettingValue && $pubIdSettingValue != EXPORT_STATUS_NOT_DEPOSITED) {
-            $params[] = $pubIdSettingValue;
-        }
-
-        $result = $this->deprecatedDao->retrieveRange(
-            $sql = 'SELECT i.*
-			FROM issues i
-				LEFT JOIN custom_issue_orders o ON (o.issue_id = i.issue_id)
-				' . ($pubIdType != null ? ' LEFT JOIN issue_settings ist ON (i.issue_id = ist.issue_id)' : '')
-                . ($pubIdSettingName != null ? ' LEFT JOIN issue_settings iss ON (i.issue_id = iss.issue_id AND iss.setting_name = ?)' : '') . '
-			WHERE
-				i.published = 1  AND i.journal_id = ?
-				' . ($pubIdType != null ? ' AND ist.setting_name = ? AND ist.setting_value IS NOT NULL' : '')
-                . (($pubIdSettingName != null && $pubIdSettingValue != null && $pubIdSettingValue == EXPORT_STATUS_NOT_DEPOSITED) ? ' AND iss.setting_value IS NULL' : '')
-                . (($pubIdSettingName != null && $pubIdSettingValue != null && $pubIdSettingValue != EXPORT_STATUS_NOT_DEPOSITED) ? ' AND iss.setting_value = ?' : '')
-                . (($pubIdSettingName != null && is_null($pubIdSettingValue)) ? ' AND (iss.setting_value IS NULL OR iss.setting_value = \'\')' : '')
-                . ' ORDER BY i.date_published DESC',
-            $params,
-            $rangeInfo
-        );
-
-        return new DAOResultFactory($result, $this, 'fromRow', [], $sql, $params, $rangeInfo);
+        $q = DB::table('issues', 'i')
+            ->leftJoin('custom_issue_orders AS o', 'o.issue_id', '=', 'i.issue_id')
+            ->when($pubIdType != null, fn (Builder $q) => $q->leftJoin('issue_settings AS ist', 'i.issue_id', '=', 'ist.issue_id'))
+            ->when($pubIdSettingName, fn (Builder $q) => $q->leftJoin('issue_settings AS iss', fn (JoinClause $j) => $j->on('i.issue_id', '=', 'iss.issue_id')->where('iss.setting_name', '=', $pubIdSettingName)))
+            ->where('i.published', '=', 1)
+            ->where('i.journal_id', '=', $contextId)
+            ->when($pubIdType != null, fn (Builder $q) => $q->where('ist.setting_name', '=', "pub-id::{$pubIdType}")->whereNotNull('ist.setting_value'))
+            ->when(
+                $pubIdSettingName,
+                fn (Builder $q) => $q->when(
+                    $pubIdSettingValue === null,
+                    fn (Builder $q) => $q->whereRaw("COALESCE(iss.setting_value, '') = ''"),
+                    fn (Builder $q) => $q->when(
+                        $pubIdSettingValue != PubObjectsExportPlugin::EXPORT_STATUS_NOT_DEPOSITED,
+                        fn (Builder $q) => $q->where('iss.setting_value', '=', $pubIdSettingValue),
+                        fn (Builder $q) => $q->whereNull('iss.setting_value')
+                    )
+                )
+            )
+            ->orderByDesc('i.date_published')
+            ->select('i.*');
+
+        $result = $this->deprecatedDao->retrieveRange($q, [], $rangeInfo);
+        return new DAOResultFactory($result, $this, 'fromRow', [], $q, [], $rangeInfo);
     }
 
     /**

diff --git a/classes/plugins/PubObjectsExportPlugin.php b/classes/plugins/PubObjectsExportPlugin.php
@@ -43,25 +43,19 @@
 use PKP\submission\PKPSubmission;
 use PKP\user\User;
 
-// The statuses.
-define('EXPORT_STATUS_ANY', '');
-define('EXPORT_STATUS_NOT_DEPOSITED', 'notDeposited');
-define('EXPORT_STATUS_MARKEDREGISTERED', 'markedRegistered');
-define('EXPORT_STATUS_REGISTERED', 'registered');
-
-// The actions.
-define('EXPORT_ACTION_EXPORT', 'export');
-define('EXPORT_ACTION_MARKREGISTERED', 'markRegistered');
-define('EXPORT_ACTION_DEPOSIT', 'deposit');
-
-// Configuration errors.
-define('EXPORT_CONFIG_ERROR_SETTINGS', 0x02);
-
 abstract class PubObjectsExportPlugin extends ImportExportPlugin
 {
+    // The statuses
+    public const EXPORT_STATUS_ANY = '';
+    public const EXPORT_STATUS_NOT_DEPOSITED = 'notDeposited';
+    public const EXPORT_STATUS_MARKEDREGISTERED = 'markedRegistered';
+    public const EXPORT_STATUS_REGISTERED = 'registered';
+    // The actions
     public const EXPORT_ACTION_EXPORT = 'export';
     public const EXPORT_ACTION_MARKREGISTERED = 'markRegistered';
     public const EXPORT_ACTION_DEPOSIT = 'deposit';
+    // Configuration errors.
+    public const EXPORT_CONFIG_ERROR_SETTINGS = 2;
 
     /** @var PubObjectCache */
     public $_cache;
@@ -169,7 +163,7 @@ public function display($args, $request)
                     }
                     $pluginSetting = $this->getSetting($context->getId(), $fieldName);
                     if (empty($pluginSetting)) {
-                        $configurationErrors[] = EXPORT_CONFIG_ERROR_SETTINGS;
+                        $configurationErrors[] = PubObjectsExportPlugin::EXPORT_CONFIG_ERROR_SETTINGS;
                         break;
                     }
                 }
@@ -252,7 +246,7 @@ public function executeExportAction($request, $objects, $filter, $tab, $objectsF
     {
         $context = $request->getContext();
         $path = ['plugin', $this->getName()];
-        if ($this->_checkForExportAction(EXPORT_ACTION_EXPORT)) {
+        if ($this->_checkForExportAction(PubObjectsExportPlugin::EXPORT_ACTION_EXPORT)) {
             assert($filter != null);
 
             $onlyValidateExport = ($request->getUserVar('onlyValidateExport')) ? true : false;
@@ -288,7 +282,7 @@ public function executeExportAction($request, $objects, $filter, $tab, $objectsF
                 $fileManager->downloadByPath($exportFileName);
                 $fileManager->deleteByPath($exportFileName);
             }
-        } elseif ($this->_checkForExportAction(EXPORT_ACTION_DEPOSIT)) {
+        } elseif ($this->_checkForExportAction(PubObjectsExportPlugin::EXPORT_ACTION_DEPOSIT)) {
             assert($filter != null);
             // Get the XML
             $exportXml = $this->exportXML($objects, $filter, $context, $noValidation);
@@ -325,7 +319,7 @@ public function executeExportAction($request, $objects, $filter, $tab, $objectsF
                 // redirect back to the right tab
                 $request->redirect(null, null, null, $path, null, $tab);
             }
-        } elseif ($this->_checkForExportAction(EXPORT_ACTION_MARKREGISTERED)) {
+        } elseif ($this->_checkForExportAction(PubObjectsExportPlugin::EXPORT_ACTION_MARKREGISTERED)) {
             $this->markRegistered($context, $objects);
             if ($shouldRedirect) {
                 // redirect back to the right tab
@@ -409,10 +403,10 @@ public function getRepresentationFilter()
     public function getStatusNames()
     {
         return [
-            EXPORT_STATUS_ANY => __('plugins.importexport.common.status.any'),
-            EXPORT_STATUS_NOT_DEPOSITED => __('plugins.importexport.common.status.notDeposited'),
-            EXPORT_STATUS_MARKEDREGISTERED => __('plugins.importexport.common.status.markedRegistered'),
-            EXPORT_STATUS_REGISTERED => __('plugins.importexport.common.status.registered'),
+            PubObjectsExportPlugin::EXPORT_STATUS_ANY => __('plugins.importexport.common.status.any'),
+            PubObjectsExportPlugin::EXPORT_STATUS_NOT_DEPOSITED => __('plugins.importexport.common.status.notDeposited'),
+            PubObjectsExportPlugin::EXPORT_STATUS_MARKEDREGISTERED => __('plugins.importexport.common.status.markedRegistered'),
+            PubObjectsExportPlugin::EXPORT_STATUS_REGISTERED => __('plugins.importexport.common.status.registered'),
         ];
     }
 
@@ -438,9 +432,9 @@ public function getStatusActions($pubObject)
      */
     public function getExportActions($context)
     {
-        $actions = [EXPORT_ACTION_EXPORT, EXPORT_ACTION_MARKREGISTERED];
+        $actions = [PubObjectsExportPlugin::EXPORT_ACTION_EXPORT, PubObjectsExportPlugin::EXPORT_ACTION_MARKREGISTERED];
         if ($this->getSetting($context->getId(), 'username') && $this->getSetting($context->getId(), 'password')) {
-            array_unshift($actions, EXPORT_ACTION_DEPOSIT);
+            array_unshift($actions, PubObjectsExportPlugin::EXPORT_ACTION_DEPOSIT);
         }
         return $actions;
     }
@@ -453,9 +447,9 @@ public function getExportActions($context)
     public function getExportActionNames()
     {
         return [
-            EXPORT_ACTION_DEPOSIT => __('plugins.importexport.common.action.register'),
-            EXPORT_ACTION_EXPORT => __('plugins.importexport.common.action.export'),
-            EXPORT_ACTION_MARKREGISTERED => __('plugins.importexport.common.action.markRegistered'),
+            PubObjectsExportPlugin::EXPORT_ACTION_DEPOSIT => __('plugins.importexport.common.action.register'),
+            PubObjectsExportPlugin::EXPORT_ACTION_EXPORT => __('plugins.importexport.common.action.export'),
+            PubObjectsExportPlugin::EXPORT_ACTION_MARKREGISTERED => __('plugins.importexport.common.action.markRegistered'),
         ];
     }
 
@@ -513,7 +507,7 @@ public function exportXML($objects, $filter, $context, $noValidation = null, &$o
     public function markRegistered($context, $objects)
     {
         foreach ($objects as $object) {
-            $object->setData($this->getDepositStatusSettingName(), EXPORT_STATUS_MARKEDREGISTERED);
+            $object->setData($this->getDepositStatusSettingName(), PubObjectsExportPlugin::EXPORT_STATUS_MARKEDREGISTERED);
             $this->updateObject($object);
         }
     }
@@ -623,7 +617,7 @@ public function getUnregisteredArticles($context)
             null,
             null,
             $this->getDepositStatusSettingName(),
-            EXPORT_STATUS_NOT_DEPOSITED,
+            PubObjectsExportPlugin::EXPORT_STATUS_NOT_DEPOSITED,
             null
         );
         return $articles->toArray();
@@ -932,4 +926,17 @@ protected function _checkForExportAction($exportAction)
 
 if (!PKP_STRICT_MODE) {
     class_alias('\APP\plugins\PubObjectsExportPlugin', '\PubObjectsExportPlugin');
+
+    foreach ([
+        'EXPORT_STATUS_ANY',
+        'EXPORT_STATUS_NOT_DEPOSITED',
+        'EXPORT_STATUS_MARKEDREGISTERED',
+        'EXPORT_STATUS_REGISTERED',
+        'EXPORT_ACTION_EXPORT',
+        'EXPORT_ACTION_MARKREGISTERED',
+        'EXPORT_ACTION_DEPOSIT',
+        'EXPORT_CONFIG_ERROR_SETTINGS',
+    ] as $constantName) {
+        define($constantName, constant('\PubObjectsExportPlugin::' . $constantName));
+    }
 }
diff --git a/classes/services/StatsIssueService.php b/classes/services/StatsIssueService.php
@@ -57,7 +57,7 @@ public function getCount(array $args): int
 
         Hook::call('StatsIssue::getCount::queryBuilder', [&$metricsQB, $args]);
 
-        return $metricsQB->getIssueIds()->get()->count();
+        return $metricsQB->getIssueIds()->getCountForPagination();
     }
 
     /**

diff --git a/classes/services/queryBuilders/GalleyQueryBuilder.php b/classes/services/queryBuilders/GalleyQueryBuilder.php
@@ -56,13 +56,11 @@ public function getCount()
     {
         return $this
             ->getQuery()
-            ->select('g.galley_id')
-            ->get()
-            ->count();
+            ->getCountForPagination();
     }
 
     /**
-     * @copydoc PKP\services\queryBuilders\interfaces\EntityQueryBuilderInterface::getCount()
+     * @copydoc PKP\services\queryBuilders\interfaces\EntityQueryBuilderInterface::getIds()
      */
     public function getIds()
     {
@@ -74,7 +72,7 @@ public function getIds()
     }
 
     /**
-     * @copydoc PKP\services\queryBuilders\interfaces\EntityQueryBuilderInterface::getCount()
+     * @copydoc PKP\services\queryBuilders\interfaces\EntityQueryBuilderInterface::getQuery()
      */
     public function getQuery()
     {

diff --git a/classes/services/queryBuilders/StatsIssueQueryBuilder.php b/classes/services/queryBuilders/StatsIssueQueryBuilder.php
@@ -69,7 +69,7 @@ public function getIssueIds(): Builder
     {
         return $this->_getObject()
             ->select([StatisticsHelper::STATISTICS_DIMENSION_ISSUE_ID])
-            ->distinct();
+            ->groupBy(StatisticsHelper::STATISTICS_DIMENSION_ISSUE_ID);
     }
 
     /**