Merge pull request #14878 from Budibase/allow-bigints-in-calculations

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

@@ -179,12 +179,6 @@ class InternalBuilder {
     return this.table.schema[column]
   }
 
-  private supportsILike(): boolean {
-    return !(
-      this.client === SqlClient.ORACLE || this.client === SqlClient.SQL_LITE
-    )
-  }
-
   private quoteChars(): [string, string] {
     const wrapped = this.knexClient.wrapIdentifier("foo", {})
     return [wrapped[0], wrapped[wrapped.length - 1]]
@@ -216,8 +210,30 @@ class InternalBuilder {
     return formatter.wrap(value, false)
   }
 
-  private rawQuotedValue(value: string): Knex.Raw {
+  private castIntToString(identifier: string | Knex.Raw): Knex.Raw {
-    return this.knex.raw(this.quotedValue(value))
+    switch (this.client) {
+      case SqlClient.ORACLE: {
+        return this.knex.raw("to_char(??)", [identifier])
+      }
+      case SqlClient.POSTGRES: {
+        return this.knex.raw("??::TEXT", [identifier])
+      }
+      case SqlClient.MY_SQL:
+      case SqlClient.MARIADB: {
+        return this.knex.raw("CAST(?? AS CHAR)", [identifier])
+      }
+      case SqlClient.SQL_LITE: {
+        // Technically sqlite can actually represent numbers larger than a 64bit
+        // int as a string, but it does it using scientific notation (e.g.
+        // "1e+20") which is not what we want. Given that the external SQL
+        // databases are limited to supporting only 64bit ints, we settle for
+        // that here.
+        return this.knex.raw("printf('%d', ??)", [identifier])
+      }
+      case SqlClient.MS_SQL: {
+        return this.knex.raw("CONVERT(NVARCHAR, ??)", [identifier])
+      }
+    }
   }
 
   // Unfortuantely we cannot rely on knex's identifier escaping because it trims
@@ -1078,21 +1094,26 @@ class InternalBuilder {
           query = query.count(`* as ${aggregation.name}`)
         }
       } else {
-        const field = `${tableName}.${aggregation.field} as ${aggregation.name}`
+        const fieldSchema = this.getFieldSchema(aggregation.field)
-        switch (op) {
+        if (!fieldSchema) {
-          case CalculationType.SUM:
+          // This should not happen in practice.
-            query = query.sum(field)
+          throw new Error(
-            break
+            `field schema missing for aggregation target: ${aggregation.field}`
-          case CalculationType.AVG:
+          )
-            query = query.avg(field)
-            break
-          case CalculationType.MIN:
-            query = query.min(field)
-            break
-          case CalculationType.MAX:
-            query = query.max(field)
-            break
         }
+
+        let aggregate = this.knex.raw("??(??)", [
+          this.knex.raw(op),
+          this.rawQuotedIdentifier(`${tableName}.${aggregation.field}`),
+        ])
+
+        if (fieldSchema.type === FieldType.BIGINT) {
+          aggregate = this.castIntToString(aggregate)
+        }
+
+        query = query.select(
+          this.knex.raw("?? as ??", [aggregate, aggregation.name])
+        )
       }
     }
     return query

packages/builder/src/components/backend/DataTable/buttons/grid/GridViewCalculationButton.svelte

@@ -7,7 +7,7 @@
     Icon,
     Multiselect,
   } from "@budibase/bbui"
-  import { CalculationType, canGroupBy, FieldType } from "@budibase/types"
+  import { CalculationType, canGroupBy, isNumeric } from "@budibase/types"
   import InfoDisplay from "pages/builder/app/[application]/design/[screenId]/[componentId]/_components/Component/InfoDisplay.svelte"
   import { getContext } from "svelte"
 
@@ -90,10 +90,7 @@
     return Object.entries(schema)
       .filter(([field, fieldSchema]) => {
         // Only allow numeric fields that are not calculations themselves
-        if (
+        if (fieldSchema.calculationType || !isNumeric(fieldSchema.type)) {
-          fieldSchema.calculationType ||
-          fieldSchema.type !== FieldType.NUMBER
-        ) {
           return false
         }
         // Don't allow duplicates

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

@@ -2268,58 +2268,118 @@ describe.each([
       })
     })
 
-    describe("calculation views", () => {
+    !isLucene &&
-      it("should not remove calculation columns when modifying table schema", async () => {
+      describe("calculation views", () => {
-        let table = await config.api.table.save(
+        it("should not remove calculation columns when modifying table schema", async () => {
-          saveTableRequest({
+          let table = await config.api.table.save(
-            schema: {
+            saveTableRequest({
-              name: {
+              schema: {
-                name: "name",
+                name: {
-                type: FieldType.STRING,
+                  name: "name",
+                  type: FieldType.STRING,
+                },
+                age: {
+                  name: "age",
+                  type: FieldType.NUMBER,
+                },
               },
-              age: {
+            })
-                name: "age",
+          )
-                type: FieldType.NUMBER,
+
+          let view = await config.api.viewV2.create({
+            tableId: table._id!,
+            name: generator.guid(),
+            type: ViewV2Type.CALCULATION,
+            schema: {
+              sum: {
+                visible: true,
+                calculationType: CalculationType.SUM,
+                field: "age",
               },
             },
           })
-        )
 
-        let view = await config.api.viewV2.create({
+          table = await config.api.table.get(table._id!)
-          tableId: table._id!,
+          await config.api.table.save({
-          name: generator.guid(),
+            ...table,
-          type: ViewV2Type.CALCULATION,
+            schema: {
-          schema: {
+              ...table.schema,
-            sum: {
+              name: {
-              visible: true,
+                name: "name",
-              calculationType: CalculationType.SUM,
+                type: FieldType.STRING,
-              field: "age",
+                constraints: { presence: true },
+              },
             },
-          },
+          })
+
+          view = await config.api.viewV2.get(view.id)
+          expect(Object.keys(view.schema!).sort()).toEqual([
+            "age",
+            "id",
+            "name",
+            "sum",
+          ])
         })
 
-        table = await config.api.table.get(table._id!)
+        describe("bigints", () => {
-        await config.api.table.save({
+          let table: Table
-          ...table,
+          let view: ViewV2
-          schema: {
-            ...table.schema,
-            name: {
-              name: "name",
-              type: FieldType.STRING,
-              constraints: { presence: true },
-            },
-          },
-        })
 
-        view = await config.api.viewV2.get(view.id)
+          beforeEach(async () => {
-        expect(Object.keys(view.schema!).sort()).toEqual([
+            table = await config.api.table.save(
-          "age",
+              saveTableRequest({
-          "id",
+                schema: {
-          "name",
+                  bigint: {
-          "sum",
+                    name: "bigint",
-        ])
+                    type: FieldType.BIGINT,
+                  },
+                },
+              })
+            )
+
+            view = await config.api.viewV2.create({
+              tableId: table._id!,
+              name: generator.guid(),
+              type: ViewV2Type.CALCULATION,
+              schema: {
+                sum: {
+                  visible: true,
+                  calculationType: CalculationType.SUM,
+                  field: "bigint",
+                },
+              },
+            })
+          })
+
+          it("should not lose precision handling ints larger than JSs int53", async () => {
+            // The sum of the following 3 numbers cannot be represented by
+            // JavaScripts default int53 datatype for numbers, so this is a test
+            // that makes sure we aren't losing precision between the DB and the
+            // user.
+            await config.api.row.bulkImport(table._id!, {
+              rows: [
+                { bigint: "1000000000000000000" },
+                { bigint: "123" },
+                { bigint: "321" },
+              ],
+            })
+
+            const { rows } = await config.api.row.search(view.id)
+            expect(rows).toHaveLength(1)
+            expect(rows[0].sum).toEqual("1000000000000000444")
+          })
+
+          it("should be able to handle up to 2**63 - 1 bigints", async () => {
+            await config.api.row.bulkImport(table._id!, {
+              rows: [{ bigint: "9223372036854775806" }, { bigint: "1" }],
+            })
+
+            const { rows } = await config.api.row.search(view.id)
+            expect(rows).toHaveLength(1)
+            expect(rows[0].sum).toEqual("9223372036854775807")
+          })
+        })
       })
-    })
   })
 
   describe("row operations", () => {

packages/server/src/utilities/rowProcessor/index.ts

@@ -440,19 +440,26 @@ export async function coreOutputProcessing(
     }
 
     if (sdk.views.isView(source)) {
-      const calculationFields = Object.keys(
+      // We ensure calculation fields are returned as numbers.  During the
-        helpers.views.calculationFields(source)
-      )
-
-      // We ensure all calculation fields are returned as numbers.  During the
       // testing of this feature it was discovered that the COUNT operation
       // returns a string for MySQL, MariaDB, and Postgres. But given that all
-      // calculation fields should be numbers, we blanket make sure of that
+      // calculation fields (except ones operating on BIGINTs) should be
-      // here.
+      // numbers, we blanket make sure of that here.
-      for (const key of calculationFields) {
+      for (const [name, field] of Object.entries(
+        helpers.views.calculationFields(source)
+      )) {
+        if ("field" in field) {
+          const targetSchema = table.schema[field.field]
+          // We don't convert BIGINT fields to floats because we could lose
+          // precision.
+          if (targetSchema.type === FieldType.BIGINT) {
+            continue
+          }
+        }
+
         for (const row of rows) {
-          if (typeof row[key] === "string") {
+          if (typeof row[name] === "string") {
-            row[key] = parseFloat(row[key])
+            row[name] = parseFloat(row[name])
           }
         }
       }