Added support for GROUP BY with ONLY_FULL_GROUP_BY enabled

Author: Dekel Barzilay

src/index.js

@@ -275,10 +275,22 @@ class Service extends AdapterService {
     }
   }
 
+  getGroupByColumns (query) {
+    for (const operation of query._operations) {
+      if (operation.name === 'groupBy') {
+        const args = operation.args;
+
+        return Array.isArray(args[0]) ? args[0] : args;
+      }
+    }
+
+    return null;
+  }
+
   _createQuery (params = {}) {
     const trx = params.transaction ? params.transaction.trx : null;
     const schema = params.schema || this.schema;
-    let query = this.Model.query(trx);
+    const query = this.Model.query(trx);
 
     return schema ? query.withSchema(schema) : query;
   }
@@ -319,10 +331,10 @@ class Service extends AdapterService {
       delete query.$joinEager;
     }
 
-    if (query && query.$joinRelation) {
-      q
-        .distinct(`${this.Model.tableName}.*`)
-        .joinRelated(query.$joinRelation);
+    const joinRelation = query && query.$joinRelation;
+
+    if (joinRelation) {
+      q.joinRelated(query.$joinRelation);
 
       delete query.$joinRelation;
     }
@@ -339,6 +351,12 @@ class Service extends AdapterService {
       delete query.$modify;
     }
 
+    if (joinRelation) {
+      const groupByColumns = this.getGroupByColumns(q);
+
+      if (!groupByColumns) { q.distinct(`${this.Model.tableName}.*`); }
+    }
+
     // apply eager filters if specified
     if (this.eagerFilters) {
       const eagerFilters = Array.isArray(this.eagerFilters) ? this.eagerFilters : [this.eagerFilters];
@@ -382,6 +400,7 @@ class Service extends AdapterService {
   _find (params) {
     const find = (params, count, filters, query) => {
       const q = params.objection || this.createQuery(params);
+      const groupByColumns = this.getGroupByColumns(q);
 
       // Handle $limit
       if (filters.$limit) {
@@ -416,18 +435,17 @@ class Service extends AdapterService {
       }
 
       if (count) {
-        const idColumns = Array.isArray(this.id) ? this.id.map(idKey => `${this.Model.tableName}.${idKey}`) : [`${this.Model.tableName}.${this.id}`];
-
+        const countColumns = groupByColumns || (Array.isArray(this.id) ? this.id.map(idKey => `${this.Model.tableName}.${idKey}`) : [`${this.Model.tableName}.${this.id}`]);
         const countQuery = this._createQuery(params);
 
         if (query.$joinRelation) {
           countQuery
             .joinRelated(query.$joinRelation)
-            .countDistinct({ total: idColumns });
-        } else if (idColumns.length > 1) {
-          countQuery.countDistinct({ total: idColumns });
+            .countDistinct({ total: countColumns });
+        } else if (countColumns.length > 1) {
+          countQuery.countDistinct({ total: countColumns });
         } else {
-          countQuery.count({ total: idColumns });
+          countQuery.count({ total: countColumns });
         }
 
         if (query && query.$modify) {

test/company.js

@@ -38,8 +38,9 @@ export default class Company extends Model {
       builder.orderBy('name');
     },
     google: (builder, hasCeo) => {
-      builder.where('name', 'Google')
-        .groupBy(['name']);
+      builder.where('companies.name', 'Google')
+        .select(['companies.name'])
+        .groupBy(['companies.name']);
 
       if (hasCeo) { builder.whereNot('ceo', null); }
     },

test/index.test.js

@@ -1701,8 +1701,10 @@ describe('Feathers Objection Service', () => {
   });
 
   describe('$modify', () => {
-    before(async () => {
-      await companies
+    const ids = {};
+
+    beforeEach(async () => {
+      ids.companies = await companies
         .create([
           {
             name: 'Google',
@@ -1715,13 +1717,25 @@ describe('Feathers Objection Service', () => {
             size: 'large'
           }
         ]);
+
+      ids.employees = await employees
+        .create([
+          {
+            name: 'John',
+            companyId: ids.companies[0].id
+          },
+          {
+            name: 'Apple',
+            companyId: ids.companies[1].id
+          }
+        ]);
     });
 
     afterEach(async () => {
       companies.options.paginate = {};
-    });
 
-    after(async () => {
+      for (const employee of ids.employees) { await employees.remove(employee.id); }
+
       await companies.remove(null);
     });
 
@@ -1757,6 +1771,19 @@ describe('Feathers Objection Service', () => {
       });
     });
 
+    it('allow $modify query with paginate and groupBy', () => {
+      companies.options.paginate = {
+        default: 1,
+        max: 2
+      };
+
+      return companies.find({ query: { $modify: ['google'], $joinRelation: 'employees' } }).then(data => {
+        expect(data.total).to.be.equal(1);
+        expect(data.data.length).to.be.equal(1);
+        expect(data.data[0].name).to.be.equal('Google');
+      });
+    });
+
     it('allow $modify query as string', () => {
       return companies.find({ query: { $modify: 'google' } }).then(data => {
         expect(data.length).to.be.equal(1);