From 4165c6cab42affee7f40f5493e29e06a90c67ec4 Mon Sep 17 00:00:00 2001 From: Sam Rose Date: Tue, 1 Oct 2024 16:06:40 +0100 Subject: [PATCH] Test all aggregation types. --- .../src/api/routes/tests/viewV2.spec.ts | 55 +++++++++++++++++++ packages/server/src/integrations/postgres.ts | 3 +- .../src/utilities/rowProcessor/index.ts | 19 +++++++ 3 files changed, 76 insertions(+), 1 deletion(-) diff --git a/packages/server/src/api/routes/tests/viewV2.spec.ts b/packages/server/src/api/routes/tests/viewV2.spec.ts index b03c445b78..1d6c1d50cd 100644 --- a/packages/server/src/api/routes/tests/viewV2.spec.ts +++ b/packages/server/src/api/routes/tests/viewV2.spec.ts @@ -2490,6 +2490,61 @@ describe.each([ expect(row["Total Price"]).toEqual(priceByQuantity[row.quantity]) } }) + + it.each([ + CalculationType.COUNT, + CalculationType.SUM, + CalculationType.AVG, + CalculationType.MIN, + CalculationType.MAX, + ])("should be able to calculate $type", async type => { + const view = await config.api.viewV2.create({ + tableId: table._id!, + name: generator.guid(), + schema: { + aggregate: { + visible: true, + calculationType: type, + field: "price", + }, + }, + }) + + const response = await config.api.viewV2.search(view.id, { + query: {}, + }) + + function calculate( + type: CalculationType, + numbers: number[] + ): number { + switch (type) { + case CalculationType.COUNT: + return numbers.length + case CalculationType.SUM: + return numbers.reduce((a, b) => a + b, 0) + case CalculationType.AVG: + return numbers.reduce((a, b) => a + b, 0) / numbers.length + case CalculationType.MIN: + return Math.min(...numbers) + case CalculationType.MAX: + return Math.max(...numbers) + } + } + + const prices = rows.map(row => row.price) + const expected = calculate(type, prices) + const actual = response.rows[0].aggregate + + if (type === CalculationType.AVG) { + // The average calculation can introduce floating point rounding + // errors, so we need to compare to within a small margin of + // error. + expect(actual).toBeCloseTo(expected) + } else { + expect(actual).toEqual(expected) + } + }) }) }) diff --git a/packages/server/src/integrations/postgres.ts b/packages/server/src/integrations/postgres.ts index 3652864991..ce8b21eede 100644 --- a/packages/server/src/integrations/postgres.ts +++ b/packages/server/src/integrations/postgres.ts @@ -272,7 +272,8 @@ class PostgresIntegration extends Sql implements DatasourcePlus { try { const bindings = query.bindings || [] this.log(query.sql, bindings) - return await client.query(query.sql, bindings) + const result = await client.query(query.sql, bindings) + return result } catch (err: any) { await this.closeConnection() let readableMessage = getReadableErrorMessage( diff --git a/packages/server/src/utilities/rowProcessor/index.ts b/packages/server/src/utilities/rowProcessor/index.ts index bddd590f25..7332f8b244 100644 --- a/packages/server/src/utilities/rowProcessor/index.ts +++ b/packages/server/src/utilities/rowProcessor/index.ts @@ -426,6 +426,25 @@ export async function coreOutputProcessing( } } } + + if (sdk.views.isView(source)) { + const calculationFields = Object.keys( + 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 + // here. + for (const key of calculationFields) { + for (const row of rows) { + if (typeof row[key] === "string") { + row[key] = parseFloat(row[key]) + } + } + } + } } if (!isUserMetadataTable(table._id!)) {