Merge pull request #1928 from bookshelf/rg-fix-pagination-groupby

Fix crash when using groupBy with table qualifier
bookshelf · Dec 17, 2018 · 62371cc · 62371cc
2 parents 68e8537 + 36b165a
commit 62371cc
Show file tree

Hide file tree

Showing 2 changed files with 20 additions and 2 deletions.
diff --git a/lib/plugins/pagination.js b/lib/plugins/pagination.js
@@ -166,8 +166,7 @@ module.exports = function paginationPlugin(bookshelf) {
           // for a count, and grouping returns the entire result set
           // What we want instead is to use `DISTINCT`
           _.remove(qb._statements, (statement) => {
-            if (statement.grouping === 'group')
-              statement.value.forEach((value) => groupColumns.push(`${tableName}.${value}`));
+            if (statement.grouping === 'group') statement.value.forEach((value) => groupColumns.push(value));
 
             return notNeededQueries.indexOf(statement.type) > -1 || statement.grouping === 'columns';
           });

diff --git a/test/integration/plugins/pagination.js b/test/integration/plugins/pagination.js
@@ -180,6 +180,25 @@ module.exports = function(bookshelf) {
             expect(blogs.length).to.be.below(total);
           });
       });
+
+      it('counts grouped rows when using table name qualifier', function() {
+        var total;
+
+        return Models.Blog.count()
+          .then(function(count) {
+            total = parseInt(count, 10);
+
+            return Models.Blog.query(function(qb) {
+              qb.max('id');
+              qb.groupBy('blogs.site_id');
+              qb.whereNotNull('site_id');
+            }).fetchPage();
+          })
+          .then(function(blogs) {
+            expect(blogs.pagination.rowCount).to.equal(blogs.length);
+            expect(blogs.length).to.be.below(total);
+          });
+      });
     });
 
     describe('with fetch Options', function() {