Skip to content

Commit

Permalink
Merge pull request #9994 from jonasraoni/feature-stable-3_4_0-8700-im…
Browse files Browse the repository at this point in the history
…prove-slow-query

Feature stable 3 4 0 8700 improve slow query
  • Loading branch information
jonasraoni authored May 30, 2024
2 parents ad4d516 + d9c1a6f commit 7d495dc
Show file tree
Hide file tree
Showing 41 changed files with 172 additions and 219 deletions.
2 changes: 1 addition & 1 deletion api/v1/_submissions/PKPBackendSubmissionsHandler.php
Original file line number Diff line number Diff line change
Expand Up @@ -134,7 +134,7 @@ public function getMany($slimRequest, $response, $args)
$genres = $genreDao->getByContextId($context->getId())->toArray();

return $response->withJson([
'itemsMax' => $collector->limit(null)->offset(null)->getCount(),
'itemsMax' => $collector->getCount(),
'items' => Repo::submission()->getSchemaMap()->mapManyToSubmissionsList($submissions, $userGroups, $genres)->values(),
], 200);
}
Expand Down
2 changes: 1 addition & 1 deletion api/v1/announcements/PKPAnnouncementHandler.php
Original file line number Diff line number Diff line change
Expand Up @@ -188,7 +188,7 @@ public function getMany($slimRequest, $response, $args)
$announcements = $collector->getMany();

return $response->withJson([
'itemsMax' => $collector->limit(null)->offset(null)->getCount(),
'itemsMax' => $collector->getCount(),
'items' => Repo::announcement()->getSchemaMap()->summarizeMany($announcements)->values(),
], 200);
}
Expand Down
2 changes: 1 addition & 1 deletion api/v1/dois/PKPDoiHandler.php
Original file line number Diff line number Diff line change
Expand Up @@ -218,7 +218,7 @@ public function getMany(SlimRequest $slimRequest, APIResponse $response, array $

return $response->withJson(
[
'itemsMax' => $collector->limit(null)->offset(0)->getCount(),
'itemsMax' => $collector->getCount(),
'items' => Repo::doi()->getSchemaMap()->summarizeMany($dois)->values(),
],
200
Expand Down
2 changes: 1 addition & 1 deletion api/v1/emailTemplates/PKPEmailTemplateHandler.php
Original file line number Diff line number Diff line change
Expand Up @@ -138,7 +138,7 @@ public function getMany(SlimRequest $slimRequest, Response $response, array $arg
$emailTemplates = $collector->getMany();

return $response->withJson([
'itemsMax' => $collector->limit(null)->offset(null)->getCount(),
'itemsMax' => $collector->getCount(),
'items' => Repo::emailTemplate()->getSchemaMap()->summarizeMany($emailTemplates),
], 200);
}
Expand Down
2 changes: 1 addition & 1 deletion api/v1/highlights/HighlightsHandler.php
Original file line number Diff line number Diff line change
Expand Up @@ -142,7 +142,7 @@ public function getMany(SlimRequest $slimRequest, APIResponse $response, array $
$highlights = $collector->getMany();

return $response->withJson([
'itemsMax' => $collector->limit(null)->offset(null)->getCount(),
'itemsMax' => $collector->getCount(),
'items' => Repo::highlight()->getSchemaMap()->summarizeMany($highlights)->values(),
], 200);
}
Expand Down
2 changes: 1 addition & 1 deletion api/v1/institutions/PKPInstitutionHandler.php
Original file line number Diff line number Diff line change
Expand Up @@ -142,7 +142,7 @@ public function getMany(SlimHttpRequest $slimRequest, APIResponse $response, arr
$institutions = $collector->getMany();

return $response->withJson([
'itemsMax' => $collector->limit(null)->offset(null)->getCount(),
'itemsMax' => $collector->getCount(),
'items' => Repo::institution()->getSchemaMap()->summarizeMany($institutions->values())->values(),
], 200);
}
Expand Down
6 changes: 3 additions & 3 deletions api/v1/submissions/PKPSubmissionHandler.php
Original file line number Diff line number Diff line change
Expand Up @@ -333,7 +333,7 @@ public function getMany($slimRequest, $response, $args)
$genres = $genreDao->getByContextId($context->getId())->toArray();

return $response->withJson([
'itemsMax' => $collector->limit(null)->offset(null)->getCount(),
'itemsMax' => $collector->getCount(),
'items' => Repo::submission()->getSchemaMap()->summarizeMany($submissions, $userGroups, $genres)->values(),
], 200);
}
Expand Down Expand Up @@ -887,7 +887,7 @@ public function getPublications($slimRequest, $response, $args)
$genres = $genreDao->getByContextId($submission->getData('contextId'))->toArray();

return $response->withJson([
'itemsMax' => $collector->limit(null)->offset(null)->getCount(),
'itemsMax' => $collector->getCount(),
'items' => Repo::publication()->getSchemaMap($submission, $userGroups, $genres)->summarizeMany($publications, $anonymize)->values(),
], 200);
}
Expand Down Expand Up @@ -1354,7 +1354,7 @@ public function getContributors($slimRequest, $response, $args)
$authors = $collector->getMany();

return $response->withJson([
'itemsMax' => $collector->limit(null)->offset(null)->getCount(),
'itemsMax' => $collector->getCount(),
'items' => Repo::author()->getSchemaMap()->summarizeMany($authors)->values(),
], 200);
}
Expand Down
4 changes: 2 additions & 2 deletions api/v1/users/PKPUserHandler.php
Original file line number Diff line number Diff line change
Expand Up @@ -150,7 +150,7 @@ public function getMany($slimRequest, $response, $args)
}

return $response->withJson([
'itemsMax' => $collector->limit(null)->offset(null)->getCount(),
'itemsMax' => $collector->getCount(),
'items' => $items,
], 200);
}
Expand Down Expand Up @@ -238,7 +238,7 @@ public function getReviewers($slimRequest, $response, $args)
}

return $response->withJson([
'itemsMax' => $collector->limit(null)->offset(null)->getCount(),
'itemsMax' => $collector->getCount(),
'items' => $items,
], 200);
}
Expand Down
3 changes: 1 addition & 2 deletions classes/announcement/DAO.php
Original file line number Diff line number Diff line change
Expand Up @@ -82,8 +82,7 @@ public function getCount(Collector $query): int
{
return $query
->getQueryBuilder()
->get('a.' . $this->primaryKeyColumn)
->count();
->getCountForPagination();
}

/**
Expand Down
2 changes: 1 addition & 1 deletion classes/author/DAO.php
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,7 @@ public function getCount(Collector $query): int
{
return $query
->getQueryBuilder()
->count();
->getCountForPagination();
}

/**
Expand Down
2 changes: 1 addition & 1 deletion classes/category/DAO.php
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ public function newDataObject(): Category
*/
public function getCount(Collector $query): int
{
return $query->getQueryBuilder()->count();
return $query->getQueryBuilder()->getCountForPagination();
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -182,7 +182,7 @@ public function getItems($request)
*/
public function getItemsMax()
{
return $this->_getCollector()->offset(null)->limit(null)->getCount();
return $this->_getCollector()->getCount();
}

/**
Expand Down
23 changes: 16 additions & 7 deletions classes/db/DAO.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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
Expand All @@ -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), $params);
return $sql instanceof Builder ? $sql->get() : DB::cursor(DB::raw($sql), $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;
}
Expand Down
13 changes: 7 additions & 6 deletions classes/db/DAOResultFactory.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -45,7 +47,7 @@ class DAOResultFactory extends ItemIterator
public $records;

/**
* @var string|null Fetch SQL
* @var string|Builder|null Fetch SQL
*/
public $sql;

Expand Down Expand Up @@ -75,8 +77,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)
Expand Down Expand Up @@ -206,9 +208,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();
}

/**
Expand Down
2 changes: 1 addition & 1 deletion classes/decision/DAO.php
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ public function getCount(Collector $query): int
{
return $query
->getQueryBuilder()
->count();
->getCountForPagination();
}

/**
Expand Down
4 changes: 1 addition & 3 deletions classes/doi/DAO.php
Original file line number Diff line number Diff line change
Expand Up @@ -78,9 +78,7 @@ public function getCount(Collector $query): int
{
return $query
->getQueryBuilder()
->select('d.' . $this->primaryKeyColumn)
->get()
->count();
->getCountForPagination();
}

/**
Expand Down
4 changes: 2 additions & 2 deletions classes/doi/Repository.php
Original file line number Diff line number Diff line change
Expand Up @@ -322,8 +322,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,
};
}
Expand Down
3 changes: 1 addition & 2 deletions classes/emailTemplate/DAO.php
Original file line number Diff line number Diff line change
Expand Up @@ -145,8 +145,7 @@ public function getCount(Collector $query): int
{
return $query
->getQueryBuilder()
->get()
->count();
->getCountForPagination();
}

/**
Expand Down
98 changes: 42 additions & 56 deletions classes/galley/DAO.php
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,10 @@
namespace PKP\galley;

use APP\facades\Repo;
use APP\plugins\PubObjectsExportPlugin;
use APP\publication\Publication;
use Illuminate\Database\Query\Builder;
use Illuminate\Database\Query\JoinClause;
use Illuminate\Support\Collection;
use Illuminate\Support\Facades\DB;
use Illuminate\Support\LazyCollection;
Expand Down Expand Up @@ -91,9 +93,7 @@ public function getCount(Collector $query): int
{
return $query
->getQueryBuilder()
->select('g.' . $this->primaryKeyColumn)
->get()
->count();
->getCountForPagination();
}

/**
Expand Down Expand Up @@ -270,59 +270,45 @@ public function deleteAllPubIds($contextId, $pubIdType)
*/
public function getExportable($contextId, $pubIdType = null, $title = null, $author = null, $issueId = null, $pubIdSettingName = null, $pubIdSettingValue = null, $rangeInfo = null)
{
$params = [];
if ($pubIdSettingName) {
$params[] = $pubIdSettingName;
}
$params[] = PKPSubmission::STATUS_PUBLISHED;
$params[] = (int) $contextId;
if ($pubIdType) {
$params[] = 'pub-id::' . $pubIdType;
}
if ($title) {
$params[] = 'title';
$params[] = '%' . $title . '%';
}
if ($author) {
array_push($params, $authorQuery = '%' . $author . '%', $authorQuery);
}
if ($issueId) {
$params[] = (int) $issueId;
}
import('classes.plugins.PubObjectsExportPlugin'); // Constant
if ($pubIdSettingName && $pubIdSettingValue && $pubIdSettingValue != EXPORT_STATUS_NOT_DEPOSITED) {
$params[] = $pubIdSettingValue;
}

$result = $this->deprecatedDao->retrieveRange(
$sql = 'SELECT g.*
FROM publication_galleys g
LEFT JOIN publications p ON (p.publication_id = g.publication_id)
LEFT JOIN publication_settings ps ON (ps.publication_id = p.publication_id)
LEFT JOIN submissions s ON (s.submission_id = p.submission_id)
LEFT JOIN submission_files sf ON (g.submission_file_id = sf.submission_file_id)
' . ($pubIdType != null ? ' LEFT JOIN publication_galley_settings gs ON (g.galley_id = gs.galley_id)' : '')
. ($title != null ? ' LEFT JOIN publication_settings pst ON (p.publication_id = pst.publication_id)' : '')
. ($author != null ? ' LEFT JOIN authors au ON (p.publication_id = au.publication_id)
LEFT JOIN author_settings asgs ON (asgs.author_id = au.author_id AND asgs.setting_name = \'' . Identity::IDENTITY_SETTING_GIVENNAME . '\')
LEFT JOIN author_settings asfs ON (asfs.author_id = au.author_id AND asfs.setting_name = \'' . Identity::IDENTITY_SETTING_FAMILYNAME . '\')
' : '')
. ($pubIdSettingName != null ? ' LEFT JOIN publication_galley_settings gss ON (g.galley_id = gss.galley_id AND gss.setting_name = ?)' : '') . '
WHERE
s.status = ? AND s.context_id = ?
' . ($pubIdType != null ? ' AND gs.setting_name = ? AND gs.setting_value IS NOT NULL' : '')
. ($title != null ? ' AND (pst.setting_name = ? AND pst.setting_value LIKE ?)' : '')
. ($author != null ? ' AND (asgs.setting_value LIKE ? OR asfs.setting_value LIKE ?)' : '')
. ($issueId != null ? ' AND (ps.setting_name = \'issueId\' AND ps.setting_value = ? AND ps.locale = \'\'' : '')
. (($pubIdSettingName != null && $pubIdSettingValue != null && $pubIdSettingValue == EXPORT_STATUS_NOT_DEPOSITED) ? ' AND gss.setting_value IS NULL' : '')
. (($pubIdSettingName != null && $pubIdSettingValue != null && $pubIdSettingValue != EXPORT_STATUS_NOT_DEPOSITED) ? ' AND gss.setting_value = ?' : '')
. (($pubIdSettingName != null && is_null($pubIdSettingValue)) ? ' AND (gss.setting_value IS NULL OR gss.setting_value = \'\')' : '') . '
GROUP BY g.galley_id
ORDER BY p.date_published DESC, p.publication_id DESC, g.galley_id DESC',
$params,
$rangeInfo
);

return new DAOResultFactory($result, $this, '_fromRow', [], $sql, $params, $rangeInfo);
$q = DB::table('publication_galleys', 'g')
->leftJoin('publications AS p', 'p.publication_id', '=', 'g.publication_id')
->leftJoin('publication_settings AS ps', 'ps.publication_id', '=', 'p.publication_id')
->leftJoin('submissions AS s', 's.submission_id', '=', 'p.submission_id')
->leftJoin('submission_files AS sf', 'g.submission_file_id', '=', 'sf.submission_file_id')
->when($pubIdType != null, fn (Builder $q) => $q->leftJoin('publication_galley_settings AS gs', 'g.galley_id', '=', 'gs.galley_id'))
->when($title != null, fn (Builder $q) => $q->leftJoin('publication_settings AS pst', 'p.publication_id', '=', 'pst.publication_id'))
->when(
$author != null,
fn (Builder $q) => $q->leftJoin('authors AS au', 'p.publication_id', '=', 'au.publication_id')
->leftJoin('author_settings AS asgs', fn (JoinClause $j) => $j->on('asgs.author_id', '=', 'au.author_id')->where('asgs.setting_name', '=', Identity::IDENTITY_SETTING_GIVENNAME))
->leftJoin('author_settings AS asfs', fn (JoinClause $j) => $j->on('asfs.author_id', '=', 'au.author_id')->where('asfs.setting_name', '=', Identity::IDENTITY_SETTING_FAMILYNAME))
)
->when($pubIdSettingName != null, fn (Builder $q) => $q->leftJoin('publication_galley_settings AS gss', fn (JoinClause $j) => $j->on('g.galley_id', '=', 'gss.galley_id')->where('gss.setting_name', '=', $pubIdSettingName)))
->where('s.status', '=', PKPSubmission::STATUS_PUBLISHED)
->where('s.context_id', '=', $contextId)
->when($pubIdType != null, fn (Builder $q) => $q->where('gs.setting_name', '=', "pub-id::{$pubIdType}")->whereNotNull('gs.setting_value'))
->when($title != null, fn (Builder $q) => $q->where('pst.setting_name', '=', 'title')->where('pst.setting_value', 'LIKE', "%{$title}%"))
->when($author != null, fn (Builder $q) => $q->whereRaw("CONCAT(COALESCE(asgs.setting_value, ''), ' ', COALESCE(asfs.setting_value, '')) LIKE ?", ["%{$author}%"]))
->when($issueId != null, fn (Builder $q) => $q->where('ps.setting_name', '=', 'issueId')->where('ps.setting_value', '=', $issueId)->where('ps.locale', '=', ''))
->when($pubIdSettingName, fn (Builder $q) =>
$q->when(
$pubIdSettingValue === null,
fn (Builder $q) => $q->whereRaw("COALESCE(gss.setting_value, '') = ''"),
fn (Builder $q) => $q->when(
$pubIdSettingValue != PubObjectsExportPlugin::EXPORT_STATUS_NOT_DEPOSITED,
fn (Builder $q) => $q->where('gss.setting_value', '=', $pubIdSettingValue),
fn (Builder $q) => $q->whereNull('gss.setting_value')
)
)
)
->groupBy('g.galley_id')
->orderByDesc('p.date_published')
->orderByDesc('p.publication_id')
->orderByDesc('g.galley_id')
->select('g.*');

$result = $this->deprecatedDao->retrieveRange($q, [], $rangeInfo);
return new DAOResultFactory($result, $this, 'fromRow', [], $q, [], $rangeInfo);
}
}
Loading

0 comments on commit 7d495dc

Please sign in to comment.