OpenSourceArk/Ghost
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Skipped separate count query in .findPage() for limit="all" requests

Browse Source 
no issue

- for large result sets or complex queries the count query itself can be quite time consuming
- when `limit: 'all'` is passed as an option there's no need to perform a separate count query because we can determine the pagination data from the final result set
- skipped count query when `limit: 'all'` option is present
- re-ordered comments to be closer to the code they reference (ie, why we have our own count query instead of Bookshelf's `.count()`
This commit is contained in:
Kevin Ansfield 2020-08-27 00:07:35 +01:00
parent e3eed7c02d
commit 5e64f113d5
3 changed files with 25 additions and 28 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

30
core/server/models/plugins/pagination.js
View File

@ -150,21 +150,25 @@ pagination = function pagination(bookshelf) {
const idAttribute = _.result(this.constructor.prototype, 'idAttribute');
const self = this;
let countPromise;
if (options.transacting) {
countPromise = this.query().clone().transacting(options.transacting).select(
bookshelf.knex.raw('count(distinct ' + tableName + '.' + idAttribute + ') as aggregate')
);
} else {
countPromise = this.query().clone().select(
bookshelf.knex.raw('count(distinct ' + tableName + '.' + idAttribute + ') as aggregate')
);
}
// #### Pre count clauses
// Add any where or join clauses which need to be included with the aggregate query
// Clone the base query & set up a promise to get the count of total items in the full set
// Due to lack of support for count distinct, this is pretty complex.
// Necessary due to lack of support for `count distinct` in bookshelf's count()
// Skipped if limit='all' as we can use the length of the fetched data set
let countPromise = Promise.resolve();
if (options.limit !== 'all') {
const countQuery = this.query().clone();
if (options.transacting) {
countQuery.transacting(options.transacting);
}
countPromise = countQuery.select(
bookshelf.knex.raw('count(distinct ' + tableName + '.' + idAttribute + ') as aggregate')
);
}
return countPromise.then(function (countResult) {
// #### Post count clauses
// Add any where or join clauses which need to NOT be included with the aggregate query
@ -197,6 +201,10 @@ pagination = function pagination(bookshelf) {
// @TODO: ensure option handling is done using an explicit pick elsewhere
return self.fetchAll(_.omit(options, ['page', 'limit']))
.then(function (fetchResult) {
if (options.limit === 'all') {
countResult = [{aggregate: fetchResult.length}];
}
return {
collection: fetchResult,
pagination: paginationUtils.formatResponse(countResult[0] ? countResult[0].aggregate : 0, options)

13
test/unit/models/post_spec.js
View File

@ -220,17 +220,10 @@ describe('Unit: models/post', function () {
}]
}
}).then(() => {
queries.length.should.eql(2);
queries[0].sql.should.eql('select count(distinct posts.id) as aggregate from `posts` where ((`posts`.`status` in (?, ?) and `posts`.`status` = ?) and (`posts`.`type` = ?))');
queries[0].bindings.should.eql([
'published',
'draft',
'published',
'post'
]);
queries.length.should.eql(1);
queries[1].sql.should.eql('select `posts`.* from `posts` where ((`posts`.`status` in (?, ?) and `posts`.`status` = ?) and (`posts`.`type` = ?)) order by CASE WHEN posts.status = \'scheduled\' THEN 1 WHEN posts.status = \'draft\' THEN 2 ELSE 3 END ASC,CASE WHEN posts.status != \'draft\' THEN posts.published_at END DESC,posts.updated_at DESC,posts.id DESC');
queries[1].bindings.should.eql([
queries[0].sql.should.eql('select `posts`.* from `posts` where ((`posts`.`status` in (?, ?) and `posts`.`status` = ?) and (`posts`.`type` = ?)) order by CASE WHEN posts.status = \'scheduled\' THEN 1 WHEN posts.status = \'draft\' THEN 2 ELSE 3 END ASC,CASE WHEN posts.status != \'draft\' THEN posts.published_at END DESC,posts.updated_at DESC,posts.id DESC');
queries[0].bindings.should.eql([
'published',
'draft',
'published',

10
test/unit/models/tag_spec.js
View File

@ -46,14 +46,10 @@ describe('Unit: models/tag', function () {
limit: 'all',
withRelated: ['count.posts']
}).then(() => {
queries.length.should.eql(2);
queries[0].sql.should.eql('select count(distinct tags.id) as aggregate from `tags` where `count`.`posts` >= ?');
queries[0].bindings.should.eql([
1
]);
queries.length.should.eql(1);
queries[1].sql.should.eql('select `tags`.*, (select count(`posts`.`id`) from `posts` left outer join `posts_tags` on `posts`.`id` = `posts_tags`.`post_id` where posts_tags.tag_id = tags.id) as `count__posts` from `tags` where `count`.`posts` >= ? order by `count__posts` DESC');
queries[1].bindings.should.eql([
queries[0].sql.should.eql('select `tags`.*, (select count(`posts`.`id`) from `posts` left outer join `posts_tags` on `posts`.`id` = `posts_tags`.`post_id` where posts_tags.tag_id = tags.id) as `count__posts` from `tags` where `count`.`posts` >= ? order by `count__posts` DESC');
queries[0].bindings.should.eql([
1
]);
});