Merge pull request #14678 from Budibase/view-calculation-sql-2

Implement `GROUP BY` for calculation views.

packages/backend-core/src/sql/sql.ts

@@ -71,18 +71,6 @@ function prioritisedArraySort(toSort: string[], priorities: string[]) {
   })
 }
 
-function getTableName(table?: Table): string | undefined {
-  // SQS uses the table ID rather than the table name
-  if (
-    table?.sourceType === TableSourceType.INTERNAL ||
-    table?.sourceId === INTERNAL_TABLE_SOURCE_ID
-  ) {
-    return table?._id
-  } else {
-    return table?.name
-  }
-}
-
 function convertBooleans(query: SqlQuery | SqlQuery[]): SqlQuery | SqlQuery[] {
   if (Array.isArray(query)) {
     return query.map((q: SqlQuery) => convertBooleans(q) as SqlQuery)
@@ -99,6 +87,13 @@ function convertBooleans(query: SqlQuery | SqlQuery[]): SqlQuery | SqlQuery[] {
   return query
 }
 
+function isSqs(table: Table): boolean {
+  return (
+    table.sourceType === TableSourceType.INTERNAL ||
+    table.sourceId === INTERNAL_TABLE_SOURCE_ID
+  )
+}
+
 class InternalBuilder {
   private readonly client: SqlClient
   private readonly query: QueryJson
@@ -180,15 +175,13 @@ class InternalBuilder {
   }
 
   private generateSelectStatement(): (string | Knex.Raw)[] | "*" {
-    const { meta, endpoint, resource, tableAliases } = this.query
+    const { meta, endpoint, resource } = this.query
 
     if (!resource || !resource.fields || resource.fields.length === 0) {
       return "*"
     }
 
-    const alias = tableAliases?.[endpoint.entityId]
-      ? tableAliases?.[endpoint.entityId]
-      : endpoint.entityId
+    const alias = this.getTableName(endpoint.entityId)
     const schema = meta.table.schema
     if (!this.isFullSelectStatementRequired()) {
       return [this.knex.raw(`${this.quote(alias)}.*`)]
@@ -813,17 +806,48 @@ class InternalBuilder {
     return query
   }
 
+  isSqs(): boolean {
+    return isSqs(this.table)
+  }
+
+  getTableName(tableOrName?: Table | string): string {
+    let table: Table
+    if (typeof tableOrName === "string") {
+      const name = tableOrName
+      if (this.query.table?.name === name) {
+        table = this.query.table
+      } else if (this.query.meta.table?.name === name) {
+        table = this.query.meta.table
+      } else if (!this.query.meta.tables?.[name]) {
+        // This can legitimately happen in custom queries, where the user is
+        // querying against a table that may not have been imported into
+        // Budibase.
+        return name
+      } else {
+        table = this.query.meta.tables[name]
+      }
+    } else if (tableOrName) {
+      table = tableOrName
+    } else {
+      table = this.table
+    }
+
+    let name = table.name
+    if (isSqs(table) && table._id) {
+      // SQS uses the table ID rather than the table name
+      name = table._id
+    }
+    const aliases = this.query.tableAliases || {}
+    return aliases[name] ? aliases[name] : name
+  }
+
   addDistinctCount(query: Knex.QueryBuilder): Knex.QueryBuilder {
-    const primary = this.table.primary
-    const aliases = this.query.tableAliases
-    const aliased =
-      this.table.name && aliases?.[this.table.name]
-        ? aliases[this.table.name]
-        : this.table.name
-    if (!primary) {
+    if (!this.table.primary) {
       throw new Error("SQL counting requires primary key to be supplied")
     }
-    return query.countDistinct(`${aliased}.${primary[0]} as total`)
+    return query.countDistinct(
+      `${this.getTableName()}.${this.table.primary[0]} as __bb_total`
+    )
   }
 
   addAggregations(
@@ -831,12 +855,14 @@ class InternalBuilder {
     aggregations: Aggregation[]
   ): Knex.QueryBuilder {
     const fields = this.query.resource?.fields || []
+    const tableName = this.getTableName()
     if (fields.length > 0) {
-      query = query.groupBy(fields.map(field => `${this.table.name}.${field}`))
+      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)
@@ -861,10 +887,7 @@ class InternalBuilder {
   addSorting(query: Knex.QueryBuilder): Knex.QueryBuilder {
     let { sort, resource } = this.query
     const primaryKey = this.table.primary
-    const tableName = getTableName(this.table)
-    const aliases = this.query.tableAliases
-    const aliased =
-      tableName && aliases?.[tableName] ? aliases[tableName] : this.table?.name
+    const aliased = this.getTableName()
     if (!Array.isArray(primaryKey)) {
       throw new Error("Sorting requires primary key to be specified for table")
     }
@@ -1508,23 +1531,40 @@ class SqlQueryBuilder extends SqlTableQueryBuilder {
     return results.length ? results : [{ [operation.toLowerCase()]: true }]
   }
 
+  private getTableName(
+    table: Table,
+    aliases?: Record<string, string>
+  ): string | undefined {
+    let name = table.name
+    if (
+      table.sourceType === TableSourceType.INTERNAL ||
+      table.sourceId === INTERNAL_TABLE_SOURCE_ID
+    ) {
+      if (!table._id) {
+        return
+      }
+      // SQS uses the table ID rather than the table name
+      name = table._id
+    }
+    return aliases?.[name] || name
+  }
+
   convertJsonStringColumns<T extends Record<string, any>>(
     table: Table,
     results: T[],
     aliases?: Record<string, string>
   ): T[] {
-    const tableName = getTableName(table)
+    const tableName = this.getTableName(table, aliases)
     for (const [name, field] of Object.entries(table.schema)) {
       if (!this._isJsonColumn(field)) {
         continue
       }
-      const aliasedTableName = (tableName && aliases?.[tableName]) || tableName
-      const fullName = `${aliasedTableName}.${name}`
+      const fullName = `${tableName}.${name}` as keyof T
       for (let row of results) {
-        if (typeof row[fullName as keyof T] === "string") {
-          row[fullName as keyof T] = JSON.parse(row[fullName])
+        if (typeof row[fullName] === "string") {
+          row[fullName] = JSON.parse(row[fullName])
         }
-        if (typeof row[name as keyof T] === "string") {
+        if (typeof row[name] === "string") {
           row[name as keyof T] = JSON.parse(row[name])
         }
       }

packages/server/src/api/routes/tests/viewV2.spec.ts

@@ -2458,6 +2458,38 @@ describe.each([
               expect("_id" in row).toBe(false)
             }
           })
+
+          it("should be able to group by a basic field", async () => {
+            const view = await config.api.viewV2.create({
+              tableId: table._id!,
+              name: generator.guid(),
+              schema: {
+                quantity: {
+                  visible: true,
+                  field: "quantity",
+                },
+                "Total Price": {
+                  visible: true,
+                  calculationType: CalculationType.SUM,
+                  field: "price",
+                },
+              },
+            })
+
+            const response = await config.api.viewV2.search(view.id, {
+              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["Total Price"]).toEqual(priceByQuantity[row.quantity])
+            }
+          })
         })
     })
 

packages/server/src/sdk/app/rows/utils.ts

@@ -57,8 +57,8 @@ export function getSQLClient(datasource: Datasource): SqlClient {
 export function processRowCountResponse(
   response: DatasourcePlusQueryResponse
 ): number {
-  if (response && response.length === 1 && "total" in response[0]) {
-    const total = response[0].total
+  if (response && response.length === 1 && "__bb_total" in response[0]) {
+    const total = response[0].__bb_total
     return typeof total === "number" ? total : parseInt(total)
   } else {
     throw new Error("Unable to count rows in query - no count response")