Merge pull request pkp#8702 from jonasraoni/feature/stable-3_3_0/8700…

…-improve-slow-query

Feature/stable 3 3 0/8700 improve slow query

api/v1/_email/PKPEmailHandler.inc.php

@@ -192,7 +192,7 @@ public function process(ServerRequestInterface $slimRequest, APIResponse $respon
 		$countRunning = Capsule::table('jobs')
 			->where('queue', $args['queueId'])
 			->whereNotNull('reserved_at')
-			->count();
+			->getCountForPagination();
 		$countPending = $this->countPending($args['queueId']);
 
 		// Don't run another job if one is already running.
@@ -230,6 +230,6 @@ function() {
 	protected function countPending(string $queueId) : int {
 		return Capsule::table('jobs')
 			->where('queue', $queueId)
-			->count();
+			->getCountForPagination();
 	}
 }

api/v1/stats/publications/PKPStatsPublicationHandler.inc.php

@@ -246,7 +246,7 @@ public function getMany($slimRequest, $response, $args) {
 		$metricsDao = \DAORegistry::getDAO('MetricsDAO'); /** @var MetricsDAO */
 		return $response->withJson([
 			'items' => $items,
-			'itemsMax' => $metricsDao->countRecords($statsQO->toSql(), $statsQO->getBindings()),
+			'itemsMax' => $metricsDao->countRecords($statsQO),
 		], 200);
 	}
 

classes/db/DAO.inc.php

@@ -29,6 +29,7 @@
 define('SORT_DIRECTION_DESC', 0x00002);
 
 use Illuminate\Database\Capsule\Manager as Capsule;
+use Illuminate\Database\Query\Builder;
 
 class DAO {
 	/**
@@ -98,11 +99,15 @@ function retrieveRange($sql, $params = [], $dbResultRange = null, $callHooks = t
 
 	/**
 	 * Count the number of records in the supplied SQL statement (with optional bind parameters parameters)
-	 * @param $sql string SQL query to be counted
-	 * @param $params array Optional SQL query bind parameters
+	 * @param $sql string|Builder SQL query to be counted
+	 * @param $params array Optional SQL query bind parameters, only used when the $sql argument is a string
 	 * @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;
 	}

classes/db/DAOResultFactory.inc.php

@@ -18,6 +18,7 @@
 import('lib.pkp.classes.core.ItemIterator');
 import('lib.pkp.classes.db.DAOResultIterator');
 
+use Illuminate\Database\Query\Builder;
 use Illuminate\Support\Enumerable;
 
 class DAOResultFactory extends ItemIterator {
@@ -37,7 +38,7 @@ class DAOResultFactory extends ItemIterator {
 	var $records;
 
 	/**
-	 * @var string|null Fetch SQL
+	 * @var string|Builder|null Fetch SQL
 	 */
 	var $sql;
 
@@ -63,7 +64,7 @@ class DAOResultFactory extends ItemIterator {
 	 * @param $dao object DAO class for factory
 	 * @param $functionName The function to call on $dao to create an object
 	 * @param $idFields array 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 $sql string 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 $sql string|Builder 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 $params array 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 $rangeInfo DBResultRange Optional pagination information. WARNING: New code should not use this.
 	 */

classes/log/EmailLogDAO.inc.php

@@ -59,19 +59,22 @@ function _getByEventType($assocType, $assocId, $eventType, $userId = null, $rang
 		];
 		if ($userId) $params[] = $userId;
 
-		$result = $this->retrieveRange(
-			$sql = 'SELECT	e.*
-			FROM	email_log e' .
-			($userId ? ' LEFT JOIN email_log_users u ON e.log_id = u.email_log_id' : '') .
-			' WHERE	e.assoc_type = ? AND
+		$baseSql = '
+			FROM email_log e' .
+			($userId ? ' LEFT JOIN email_log_users u ON e.log_id = u.email_log_id' : '') . '
+			WHERE e.assoc_type = ? AND
 				e.assoc_id = ? AND
 				e.event_type = ?' .
-				($userId ? ' AND u.user_id = ?' : ''),
+				($userId ? ' AND u.user_id = ?' : '') . '
+		';
+
+		$result = $this->retrieveRange(
+			"SELECT e.* {$baseSql}",
 			$params,
 			$rangeInfo
 		);
 
-		return new DAOResultFactory($result, $this, 'build', [], $sql, $params, $rangeInfo); // Counted in submissionEmails.tpl
+		return new DAOResultFactory($result, $this, 'build', [], "SELECT 0 {$baseSql}", $params, $rangeInfo); // Counted in submissionEmails.tpl
 	}
 
 	/**

classes/migration/upgrade/PKPv3_3_0UpgradeMigration.inc.php

@@ -491,7 +491,7 @@ function (Builder $q) use ($row, $lastInsertedFileId) {
 			// be unique.
 			$count = Capsule::table('review_round_files')
 				->where('file_id', '=', $row->file_id)
-				->count();
+				->getCountForPagination();
 			if ($count > 1) {
 				Capsule::table('review_round_files')
 					->where('file_id', '=', $row->file_id)

classes/note/NoteDAO.inc.php

@@ -89,18 +89,19 @@ function getByAssoc($assocType, $assocId, $userId = null, $orderBy = NOTE_ORDER_
 				$directionSanitized = 'DESC';
 		}
 
-		$result = $this->retrieve(
-			$sql = 'SELECT	*
-			FROM	notes
-			WHERE	assoc_id = ?
+		$baseSql = '
+			FROM notes
+			WHERE assoc_id = ?
 				AND assoc_type = ?
 				' . ($userId?' AND user_id = ?':'') .
-				($isAdmin?'':'
-				AND (title IS NOT NULL OR contents IS NOT NULL)') . '
-			ORDER BY ' . $orderSanitized . ' ' . $directionSanitized,
+				($isAdmin?'':' AND (title IS NOT NULL OR contents IS NOT NULL)') . '
+		';
+
+		$result = $this->retrieve(
+			"SELECT * {$baseSql} ORDER BY {$orderSanitized} {$directionSanitized}",
 			$params
 		);
-		return new DAOResultFactory($result, $this, '_fromRow', [], $sql, $params); // Counted in QueriesGridCellProvider
+		return new DAOResultFactory($result, $this, '_fromRow', [], "SELECT 0 {$baseSql}", $params); // Counted in QueriesGridCellProvider
 	}
 
 	/**

classes/security/UserGroupDAO.inc.php

@@ -250,18 +250,21 @@ function isDefault($userGroupId) {
 	function getByRoleId($contextId, $roleId, $default = false, $dbResultRange = null) {
 		$params = [(int) $contextId, (int) $roleId];
 		if ($default) $params[] = 1; // true
-		$result = $this->retrieveRange(
-			$sql = 'SELECT	*
-			FROM	user_groups
-			WHERE	context_id = ? AND
+
+		$baseSql = '
+			FROM user_groups
+			WHERE context_id = ? AND
 				role_id = ?
-				' . ($default?' AND is_default = ?':'')
-			. ' ORDER BY user_group_id',
+				' . ($default?' AND is_default = ?':'') . '
+		';
+
+		$result = $this->retrieveRange(
+			"SELECT * {$baseSql} ORDER BY user_group_id",
 			$params,
 			$dbResultRange
 		);
 
-		return new DAOResultFactory($result, $this, '_returnFromRow', [], $sql, $params, $dbResultRange);
+		return new DAOResultFactory($result, $this, '_returnFromRow', [], "SELECT 0 {$baseSql}", $params, $dbResultRange);
 	}
 
 	/**
@@ -381,15 +384,18 @@ function getByContextId($contextId = null, $dbResultRange = null) {
 		$params = [];
 		if ($contextId) $params[] = (int) $contextId;
 
+		$baseSql = '
+			FROM user_groups ug' .
+			($contextId?' WHERE ug.context_id = ?':'') . '
+		';
+
 		$result = $this->retrieveRange(
-			$sql = 'SELECT ug.*
-			FROM	user_groups ug' .
-				($contextId?' WHERE ug.context_id = ?':''),
+			"SELECT ug.* {$baseSql}",
 			$params,
 			$dbResultRange
 		);
 
-		return new DAOResultFactory($result, $this, '_returnFromRow', [], $sql, $params, $dbResultRange);
+		return new DAOResultFactory($result, $this, '_returnFromRow', [], "SELECT 0 {$baseSql}", $params, $dbResultRange);
 	}
 
 	/**
@@ -509,14 +515,15 @@ function getUsersById($userGroupId = null, $contextId = null, $searchType = null
 				COALESCE(us.setting_value, '') = '', us.locale <> ?
 			LIMIT 1
 		)";
-		$params = [
+		$selectParams = [
 			IDENTITY_SETTING_GIVENNAME, $locale, $primaryLocale, $locale,
 			IDENTITY_SETTING_FAMILYNAME, $locale, $primaryLocale, $locale
 		];
 
-		$sql = "SELECT u.*, $settingValue AS user_given, $settingValue AS user_family
+		$baseSql = '
 			FROM users AS u
-			WHERE 1 = 1";
+			WHERE 1 = 1
+		';
 
 		// Has user group
 		if ($contextId || $userGroupId) {
@@ -526,7 +533,7 @@ function getUsersById($userGroupId = null, $contextId = null, $searchType = null
 			if ($userGroupId) {
 				$params[] = (int) $userGroupId;
 			}
-			$sql .= ' AND EXISTS (
+			$baseSql .= ' AND EXISTS (
 				SELECT 0
 				FROM user_user_groups uug
 				INNER JOIN user_groups ug
@@ -537,12 +544,16 @@ function getUsersById($userGroupId = null, $contextId = null, $searchType = null
 					' . ($userGroupId ? 'AND ug.user_group_id = ?' : '') . '
 			)';
 		}
-		$sql .= ' ' . $this->_getSearchSql($searchType, $search, $searchMatch, $params);
+		$baseSql .= ' ' . $this->_getSearchSql($searchType, $search, $searchMatch, $params);
 
 		// Get the result set
-		$result = $this->retrieveRange($sql, $params, $dbResultRange);
+		$result = $this->retrieveRange(
+			"SELECT u.*, $settingValue AS user_given, $settingValue AS user_family {$baseSql} " . $this->userDao->getOrderBy(),
+			array_merge($selectParams, $params),
+			$dbResultRange
+		);
 
-		return new DAOResultFactory($result, $this->userDao, '_returnUserFromRowWithData', [], $sql, $params, $dbResultRange);
+		return new DAOResultFactory($result, $this->userDao, '_returnUserFromRowWithData', [], "SELECT 0 {$baseSql}", $params, $dbResultRange);
 	}
 
 	//
@@ -910,8 +921,6 @@ function _getSearchSql($searchType, $search, $searchMatch, &$params) {
 			}
 		}
 
-		$searchSql .= $this->userDao->getOrderBy(); // FIXME Add "sort field" parameter?
-
 		return $searchSql;
 	}
 
@@ -930,22 +939,25 @@ function _getSearchSql($searchType, $search, $searchMatch, &$params) {
 	function getUserGroupsByStage($contextId, $stageId, $roleId = null, $dbResultRange = null) {
 		$params = [(int) $contextId, (int) $stageId];
 		if ($roleId) $params[] = (int) $roleId;
+
+		$baseSql = '
+			FROM user_groups ug
+			JOIN user_group_stage ugs ON (ug.user_group_id = ugs.user_group_id AND ug.context_id = ugs.context_id)
+			WHERE ugs.context_id = ? AND
+				ugs.stage_id = ?
+				' . ($roleId?'AND ug.role_id = ?':'') . '
+		';
+
 		return new DAOResultFactory(
 			$this->retrieveRange(
-				$sql = 'SELECT	ug.*
-				FROM	user_groups ug
-					JOIN user_group_stage ugs ON (ug.user_group_id = ugs.user_group_id AND ug.context_id = ugs.context_id)
-				WHERE	ugs.context_id = ? AND
-					ugs.stage_id = ?
-					' . ($roleId?'AND ug.role_id = ?':'') . '
-				ORDER BY ug.role_id ASC',
+				"SELECT ug.* {$baseSql} ORDER BY ug.role_id ASC",
 				$params,
 				$dbResultRange
 			),
 			$this,
 			'_returnFromRow',
 			[],
-			$sql, $params, $dbResultRange
+			"SELECT 0 {$baseSql}", $params, $dbResultRange
 		);
 	}
 

classes/services/PKPSubmissionService.inc.php

@@ -102,8 +102,8 @@ public function getMany($args = []) {
 		if (isset($args['offset'])) unset($args['offset']);
 		$submissionListQO = $this->getQueryBuilder($args)->getQuery();
 		$submissionDao = DAORegistry::getDAO('SubmissionDAO'); /* @var $submissionDao SubmissionDAO */
-		$result = $submissionDao->retrieveRange($sql = $submissionListQO->toSql(), $params = $submissionListQO->getBindings(), $range);
-		$queryResults = new DAOResultFactory($result, $submissionDao, '_fromRow', [], $sql, $params, $range);
+		$result = $submissionDao->retrieveRange($submissionListQO->toSql(), $submissionListQO->getBindings(), $range);
+		$queryResults = new DAOResultFactory($result, $submissionDao, '_fromRow', [], $submissionListQO, [], $range);
 
 		return $queryResults->toIterator();
 	}

classes/services/PKPUserService.inc.php

@@ -83,7 +83,7 @@ public function getMany($args = []) {
 		$userListQO = $this->getQueryBuilder($args)->getQuery();
 		$userDao = DAORegistry::getDAO('UserDAO'); /* @var $userDao UserDAO */
 		$result = $userDao->retrieveRange($userListQO->toSql(), $userListQO->getBindings(), $range);
-		$queryResults = new DAOResultFactory($result, $userDao, '_returnUserFromRowWithData', [], $userListQO->toSql(), $userListQO->getBindings());
+		$queryResults = new DAOResultFactory($result, $userDao, '_returnUserFromRowWithData', [], $userListQO);
 
 		return $queryResults->toIterator();
 	}
@@ -592,8 +592,9 @@ public function getAccessibleWorkflowStages($userId, $contextId, &$submission, $
 	 * @param array $args See self::getMany()
 	 */
 	public function count($args = []) {
-		$qb = $this->getQueryBuilder($args);
-		return $qb->getQuery()->get()->count();
+		return $this->getQueryBuilder($args)
+			->getQuery()
+			->getCountForPagination();
 	}
 
 	/**

classes/services/queryBuilders/PKPAnnouncementQueryBuilder.inc.php

@@ -70,9 +70,7 @@ public function searchPhrase($phrase) {
 	public function getCount() {
 		return $this
 			->getQuery()
-			->select('a.announcement_id')
-			->get()
-			->count();
+			->getCountForPagination();
 	}
 
 	/**

classes/services/queryBuilders/PKPAuthorQueryBuilder.inc.php

@@ -102,9 +102,7 @@ public function filterByPublicationIds($publicationIds) {
 	public function getCount() {
 		return $this
 			->getQuery()
-			->select('a.author_id')
-			->get()
-			->count();
+			->getCountForPagination();
 	}
 
 	/**

classes/services/queryBuilders/PKPContextQueryBuilder.inc.php

@@ -85,9 +85,7 @@ public function searchPhrase($phrase) {
 	public function getCount() {
 		return $this
 			->getQuery()
-			->select('c.' . $this->dbIdColumn)
-			->get()
-			->count();
+			->getCountForPagination();
 	}
 
 	/**

classes/services/queryBuilders/PKPEmailTemplateQueryBuilder.inc.php

@@ -179,7 +179,7 @@ public function getCount() {
 		$compiledQuery = $this->getCompiledQuery();
 		return Capsule::table(Capsule::raw('(' . $compiledQuery[0] . ') as email_template_count'))
 			->setBindings($compiledQuery[1])
-			->count();
+			->getCountForPagination();
 	}
 
 	/**

classes/services/queryBuilders/PKPPublicationQueryBuilder.inc.php

@@ -85,9 +85,7 @@ public function offsetBy($offset) {
 	public function getCount() {
 		return $this
 			->getQuery()
-			->select('p.publication_id')
-			->get()
-			->count();
+			->getCountForPagination();
 	}
 
 	/**
@@ -188,6 +186,6 @@ public function isDuplicateUrlPath($urlPath, $submissionId, $contextId) {
 			->where('url_path', '=' , $urlPath)
 			->where('p.submission_id', '!=', $submissionId)
 			->where('s.context_id', '=', $contextId)
-			->count();
+			->getCountForPagination();
 	}
 }