diff --git a/packages/backend-core/src/sql/sql.ts b/packages/backend-core/src/sql/sql.ts index f62bb0f7e3d..7eeeb5cca06 100644 --- a/packages/backend-core/src/sql/sql.ts +++ b/packages/backend-core/src/sql/sql.ts @@ -1091,7 +1091,14 @@ class InternalBuilder { ) } } else { - query = query.count(`* as ${aggregation.name}`) + if (this.client === SqlClient.ORACLE) { + const field = this.convertClobs(`${tableName}.${aggregation.field}`) + query = query.select( + this.knex.raw(`COUNT(??) as ??`, [field, aggregation.name]) + ) + } else { + query = query.count(`${aggregation.field} as ${aggregation.name}`) + } } } else { const fieldSchema = this.getFieldSchema(aggregation.field) diff --git a/packages/server/src/api/controllers/view/viewsV2.ts b/packages/server/src/api/controllers/view/viewsV2.ts index f864df9e9ec..753125e0f6d 100644 --- a/packages/server/src/api/controllers/view/viewsV2.ts +++ b/packages/server/src/api/controllers/view/viewsV2.ts @@ -12,6 +12,8 @@ import { RelationSchemaField, ViewFieldMetadata, CalculationType, + CountDistinctCalculationFieldMetadata, + CountCalculationFieldMetadata, } from "@budibase/types" import { builderSocket, gridSocket } from "../../../websockets" import { helpers } from "@budibase/shared-core" @@ -22,27 +24,31 @@ function stripUnknownFields( if (helpers.views.isCalculationField(field)) { if (field.calculationType === CalculationType.COUNT) { if ("distinct" in field && field.distinct) { - return { - order: field.order, - width: field.width, - visible: field.visible, - readonly: field.readonly, - icon: field.icon, - distinct: field.distinct, - calculationType: field.calculationType, - field: field.field, - columns: field.columns, - } + const strippedField: RequiredKeys = + { + order: field.order, + width: field.width, + visible: field.visible, + readonly: field.readonly, + icon: field.icon, + distinct: field.distinct, + calculationType: field.calculationType, + field: field.field, + columns: field.columns, + } + return strippedField } else { - return { + const strippedField: RequiredKeys = { order: field.order, width: field.width, visible: field.visible, readonly: field.readonly, icon: field.icon, calculationType: field.calculationType, + field: field.field, columns: field.columns, } + return strippedField } } const strippedField: RequiredKeys = { diff --git a/packages/server/src/api/routes/tests/viewV2.spec.ts b/packages/server/src/api/routes/tests/viewV2.spec.ts index 1f2470f097e..415b22d4072 100644 --- a/packages/server/src/api/routes/tests/viewV2.spec.ts +++ b/packages/server/src/api/routes/tests/viewV2.spec.ts @@ -693,6 +693,12 @@ describe.each([ calculationType: CalculationType.COUNT, field: "Price", }, + countDistinct: { + visible: true, + calculationType: CalculationType.COUNT, + distinct: true, + field: "Price", + }, min: { visible: true, calculationType: CalculationType.MIN, @@ -708,11 +714,6 @@ describe.each([ calculationType: CalculationType.AVG, field: "Price", }, - sum2: { - visible: true, - calculationType: CalculationType.SUM, - field: "Price", - }, }, }, { @@ -763,10 +764,12 @@ describe.each([ count: { visible: true, calculationType: CalculationType.COUNT, + field: "Price", }, count2: { visible: true, calculationType: CalculationType.COUNT, + field: "Price", }, }, }, @@ -774,7 +777,7 @@ describe.each([ status: 400, body: { message: - 'Duplicate calculation on field "*", calculation type "count"', + 'Duplicate calculation on field "Price", calculation type "count"', }, } ) @@ -805,7 +808,7 @@ describe.each([ status: 400, body: { message: - 'Duplicate calculation on field "Price", calculation type "count"', + 'Duplicate calculation on field "Price", calculation type "count distinct"', }, } ) @@ -820,6 +823,7 @@ describe.each([ count: { visible: true, calculationType: CalculationType.COUNT, + field: "Price", }, count2: { visible: true, @@ -831,6 +835,26 @@ describe.each([ }) }) + it("does not confuse counts on different fields in the duplicate check", async () => { + await config.api.viewV2.create({ + tableId: table._id!, + name: generator.guid(), + type: ViewV2Type.CALCULATION, + schema: { + count: { + visible: true, + calculationType: CalculationType.COUNT, + field: "Price", + }, + count2: { + visible: true, + calculationType: CalculationType.COUNT, + field: "Category", + }, + }, + }) + }) + !isLucene && it("does not get confused when a calculation field shadows a basic one", async () => { const table = await config.api.table.save( @@ -1607,6 +1631,7 @@ describe.each([ view.schema!.count = { visible: true, calculationType: CalculationType.COUNT, + field: "age", } await config.api.viewV2.update(view) @@ -3761,6 +3786,51 @@ describe.each([ expect(rows[0].sum).toEqual(55) }) + it("should be able to count non-numeric fields", async () => { + const table = await config.api.table.save( + saveTableRequest({ + schema: { + firstName: { + type: FieldType.STRING, + name: "firstName", + }, + lastName: { + type: FieldType.STRING, + name: "lastName", + }, + }, + }) + ) + + const view = await config.api.viewV2.create({ + tableId: table._id!, + name: generator.guid(), + type: ViewV2Type.CALCULATION, + schema: { + count: { + visible: true, + calculationType: CalculationType.COUNT, + field: "firstName", + }, + }, + }) + + await config.api.row.bulkImport(table._id!, { + rows: [ + { firstName: "Jane", lastName: "Smith" }, + { firstName: "Jane", lastName: "Doe" }, + { firstName: "Alice", lastName: "Smith" }, + ], + }) + + const { rows } = await config.api.viewV2.search(view.id, { + query: {}, + }) + + expect(rows).toHaveLength(1) + expect(rows[0].count).toEqual(3) + }) + it("should be able to filter rows on the view itself", async () => { const table = await config.api.table.save( saveTableRequest({ diff --git a/packages/server/src/sdk/app/rows/search/internal/sqs.ts b/packages/server/src/sdk/app/rows/search/internal/sqs.ts index e3bbca271e8..a160bfbb3e1 100644 --- a/packages/server/src/sdk/app/rows/search/internal/sqs.ts +++ b/packages/server/src/sdk/app/rows/search/internal/sqs.ts @@ -351,6 +351,7 @@ export async function search( aggregations.push({ name: key, calculationType: field.calculationType, + field: mapToUserColumn(field.field), }) } } else { diff --git a/packages/server/src/sdk/app/views/index.ts b/packages/server/src/sdk/app/views/index.ts index cbf3513dd09..a259dd5505f 100644 --- a/packages/server/src/sdk/app/views/index.ts +++ b/packages/server/src/sdk/app/views/index.ts @@ -61,42 +61,77 @@ export function isView(view: any): view is ViewV2 { return view.id && docIds.isViewId(view.id) && view.version === 2 } -async function guardCalculationViewSchema( - table: Table, - view: Omit -) { +function guardDuplicateCalculationFields(view: Omit) { + const seen: Record> = {} const calculationFields = helpers.views.calculationFields(view) + for (const name of Object.keys(calculationFields)) { + const schema = calculationFields[name] + const isCount = schema.calculationType === CalculationType.COUNT + const isDistinct = "distinct" in schema - if (Object.keys(calculationFields).length > 5) { - throw new HTTPError( - "Calculation views can only have a maximum of 5 fields", - 400 - ) + if (isCount && isDistinct) { + // We do these separately. + continue + } + + if (seen[schema.field]?.[schema.calculationType]) { + throw new HTTPError( + `Duplicate calculation on field "${schema.field}", calculation type "${schema.calculationType}"`, + 400 + ) + } + seen[schema.field] ??= {} as Record + seen[schema.field][schema.calculationType] = true } +} +function guardDuplicateCountDistinctFields( + view: Omit +) { const seen: Record> = {} - + const calculationFields = helpers.views.calculationFields(view) for (const name of Object.keys(calculationFields)) { const schema = calculationFields[name] const isCount = schema.calculationType === CalculationType.COUNT - const isDistinct = isCount && "distinct" in schema && schema.distinct + if (!isCount) { + continue + } - const field = isCount && !isDistinct ? "*" : schema.field - if (seen[field]?.[schema.calculationType]) { + const isDistinct = "distinct" in schema + if (!isDistinct) { + // We do these separately. + continue + } + + if (seen[schema.field]?.[schema.calculationType]) { throw new HTTPError( - `Duplicate calculation on field "${field}", calculation type "${schema.calculationType}"`, + `Duplicate calculation on field "${schema.field}", calculation type "${schema.calculationType} distinct"`, 400 ) } - seen[field] ??= {} as Record - seen[field][schema.calculationType] = true + seen[schema.field] ??= {} as Record + seen[schema.field][schema.calculationType] = true + } +} - // Count fields that aren't distinct don't need to reference another field, - // so we don't validate it. - if (isCount && !isDistinct) { - continue - } +async function guardCalculationViewSchema( + table: Table, + view: Omit +) { + const calculationFields = helpers.views.calculationFields(view) + + guardDuplicateCalculationFields(view) + guardDuplicateCountDistinctFields(view) + + if (Object.keys(calculationFields).length > 5) { + throw new HTTPError( + "Calculation views can only have a maximum of 5 fields", + 400 + ) + } + for (const name of Object.keys(calculationFields)) { + const schema = calculationFields[name] if (!schema.field) { throw new HTTPError( `Calculation field "${name}" is missing a "field" property`, @@ -112,6 +147,7 @@ async function guardCalculationViewSchema( ) } + const isCount = schema.calculationType === CalculationType.COUNT if (!isCount && !isNumeric(targetSchema.type)) { throw new HTTPError( `Calculation field "${name}" references field "${schema.field}" which is not a numeric field`, diff --git a/packages/types/src/documents/app/view.ts b/packages/types/src/documents/app/view.ts index 3ae120fccac..1212031f24e 100644 --- a/packages/types/src/documents/app/view.ts +++ b/packages/types/src/documents/app/view.ts @@ -54,12 +54,12 @@ export interface NumericCalculationFieldMetadata export interface CountCalculationFieldMetadata extends BasicViewFieldMetadata { calculationType: CalculationType.COUNT + field: string } export interface CountDistinctCalculationFieldMetadata extends CountCalculationFieldMetadata { distinct: true - field: string } export type ViewCalculationFieldMetadata = diff --git a/packages/types/src/sdk/row.ts b/packages/types/src/sdk/row.ts index ea46e825082..6328a80ba16 100644 --- a/packages/types/src/sdk/row.ts +++ b/packages/types/src/sdk/row.ts @@ -18,11 +18,11 @@ export interface NumericAggregation extends BaseAggregation { export interface CountAggregation extends BaseAggregation { calculationType: CalculationType.COUNT + field: string } export interface CountDistinctAggregation extends CountAggregation { distinct: true - field: string } export type Aggregation =