From ae4f7ae4b45ab4e30778eddff0938017924a22d0 Mon Sep 17 00:00:00 2001
From: Sam Rose <hello@samwho.dev>
Date: Tue, 1 Oct 2024 15:04:01 +0100
Subject: [PATCH] Implement group by and add a test for it.

---
 packages/backend-core/src/sql/sql.ts          | 19 +++++++++------
 .../src/api/routes/tests/viewV2.spec.ts       | 23 +++++++++++--------
 2 files changed, 26 insertions(+), 16 deletions(-)

diff --git a/packages/backend-core/src/sql/sql.ts b/packages/backend-core/src/sql/sql.ts
index 54ca5a0135..14e32623e3 100644
--- a/packages/backend-core/src/sql/sql.ts
+++ b/packages/backend-core/src/sql/sql.ts
@@ -799,6 +799,14 @@ class InternalBuilder {
     return query
   }
 
+  isSqs(t?: Table): boolean {
+    const table = t || this.table
+    return (
+      table.sourceType === TableSourceType.INTERNAL ||
+      table.sourceId === INTERNAL_TABLE_SOURCE_ID
+    )
+  }
+
   getTableName(t?: Table | string): string {
     let table: Table
     if (typeof t === "string") {
@@ -813,11 +821,7 @@ class InternalBuilder {
     }
 
     let name = table.name
-    if (
-      (table.sourceType === TableSourceType.INTERNAL ||
-        table.sourceId === INTERNAL_TABLE_SOURCE_ID) &&
-      table._id
-    ) {
+    if (this.isSqs(table) && table._id) {
       // SQS uses the table ID rather than the table name
       name = table._id
     }
@@ -830,7 +834,7 @@ class InternalBuilder {
       throw new Error("SQL counting requires primary key to be supplied")
     }
     return query.countDistinct(
-      `${this.getTableName(this.table)}.${this.table.primary[0]} as total`
+      `${this.getTableName()}.${this.table.primary[0]} as total`
     )
   }
 
@@ -842,10 +846,11 @@ class InternalBuilder {
     const tableName = this.getTableName()
     if (fields.length > 0) {
       query = query.groupBy(fields.map(field => `${tableName}.${field}`))
+      query = query.select(fields.map(field => `${tableName}.${field}`))
     }
     for (const aggregation of aggregations) {
       const op = aggregation.calculationType
-      const field = `${this.table.name}.${aggregation.field} as ${aggregation.name}`
+      const field = `${tableName}.${aggregation.field} as ${aggregation.name}`
       switch (op) {
         case CalculationType.COUNT:
           query = query.count(field)
diff --git a/packages/server/src/api/routes/tests/viewV2.spec.ts b/packages/server/src/api/routes/tests/viewV2.spec.ts
index 0f4e6c961c..b03c445b78 100644
--- a/packages/server/src/api/routes/tests/viewV2.spec.ts
+++ b/packages/server/src/api/routes/tests/viewV2.spec.ts
@@ -39,13 +39,13 @@ import {
 } from "@budibase/backend-core"
 
 describe.each([
-  // ["lucene", undefined],
-  // ["sqs", undefined],
+  ["lucene", undefined],
+  ["sqs", undefined],
   [DatabaseName.POSTGRES, getDatasource(DatabaseName.POSTGRES)],
-  // [DatabaseName.MYSQL, getDatasource(DatabaseName.MYSQL)],
-  // [DatabaseName.SQL_SERVER, getDatasource(DatabaseName.SQL_SERVER)],
-  // [DatabaseName.MARIADB, getDatasource(DatabaseName.MARIADB)],
-  // [DatabaseName.ORACLE, getDatasource(DatabaseName.ORACLE)],
+  [DatabaseName.MYSQL, getDatasource(DatabaseName.MYSQL)],
+  [DatabaseName.SQL_SERVER, getDatasource(DatabaseName.SQL_SERVER)],
+  [DatabaseName.MARIADB, getDatasource(DatabaseName.MARIADB)],
+  [DatabaseName.ORACLE, getDatasource(DatabaseName.ORACLE)],
 ])("/v2/views (%s)", (name, dsProvider) => {
   const config = setup.getConfig()
   const isSqs = name === "sqs"
@@ -2459,7 +2459,7 @@ describe.each([
             }
           })
 
-          it.only("should be able to group by a basic field", async () => {
+          it("should be able to group by a basic field", async () => {
             const view = await config.api.viewV2.create({
               tableId: table._id!,
               name: generator.guid(),
@@ -2480,9 +2480,14 @@ describe.each([
               query: {},
             })
 
+            const priceByQuantity: Record<number, number> = {}
+            for (const row of rows) {
+              priceByQuantity[row.quantity] ??= 0
+              priceByQuantity[row.quantity] += row.price
+            }
+
             for (const row of response.rows) {
-              expect(row["quantity"]).toBeGreaterThan(0)
-              expect(row["Total Price"]).toBeGreaterThan(0)
+              expect(row["Total Price"]).toEqual(priceByQuantity[row.quantity])
             }
           })
         })