Merge pull request #2081 from teableio/fix/multiple-value-rollup

feat: enhance SQL logging and improve numeric expression handling in query dialect

Author: nichenqin

apps/nestjs-backend/src/db-provider/generated-column-query/postgres/generated-column-query.postgres.ts

@@ -137,18 +137,17 @@ export class GeneratedColumnQueryPostgres extends GeneratedColumnQueryAbstract {
     }
 
     const wrapped = `(${value})`;
+    const jsonbValue = `to_jsonb${wrapped}`;
     const coerced = `(CASE
       WHEN ${wrapped} IS NULL THEN NULL
-      WHEN pg_typeof(${wrapped}) = ANY(ARRAY['jsonb'::regtype, 'json'::regtype]) THEN (
-        CASE jsonb_typeof((${wrapped})::jsonb)
-          WHEN 'string' THEN (${wrapped})::jsonb #>> '{}'
-          WHEN 'number' THEN (${wrapped})::jsonb #>> '{}'
-          WHEN 'boolean' THEN (${wrapped})::jsonb #>> '{}'
+      ELSE
+        CASE jsonb_typeof(${jsonbValue})
+          WHEN 'string' THEN ${jsonbValue} #>> '{}'
+          WHEN 'number' THEN ${jsonbValue} #>> '{}'
+          WHEN 'boolean' THEN ${jsonbValue} #>> '{}'
           WHEN 'null' THEN NULL
-          ELSE (${wrapped})::jsonb::text
+          ELSE ${jsonbValue}::text
         END
-      )
-      ELSE ${wrapped}::text
     END)`;
     return this.ensureTextCollation(coerced);
   }

apps/nestjs-backend/src/features/record/computed/services/computed-dependency-collector.service.ts

@@ -388,6 +388,7 @@ export class ComputedDependencyCollectorService {
 
     const unionQuery = this.knex.queryBuilder().union(selects);
     const finalQuery = this.knex.select('id').from(unionQuery.as('u')).distinct('id').toQuery();
+    this.logger.debug(`Linked Record IDs SQL: ${finalQuery}`);
     const rows = await this.prismaService.txClient().$queryRawUnsafe<{ id: string }[]>(finalQuery);
     return rows.map((r) => r.id).filter(Boolean);
   }

apps/nestjs-backend/src/features/record/computed/services/record-computed-update.service.ts

@@ -1,4 +1,5 @@
 import { Injectable, Logger } from '@nestjs/common';
+import { Prisma } from '@prisma/client';
 import { FieldType } from '@teable/core';
 import { PrismaService } from '@teable/db-main-prisma';
 import type { Knex } from 'knex';
@@ -102,12 +103,17 @@ export class RecordComputedUpdateService {
     const columnNames = this.getUpdatableColumns(fields);
     const returningNames = this.getReturningColumns(fields);
     if (!columnNames.length) {
+      const selectSql = qb.toQuery();
       // No updatable columns (e.g., all are generated formulas). Return current values via SELECT.
-      return await this.prismaService
-        .txClient()
-        .$queryRawUnsafe<
-          Array<{ __id: string; __version: number } & Record<string, unknown>>
-        >(qb.toQuery());
+      try {
+        return await this.prismaService
+          .txClient()
+          .$queryRawUnsafe<
+            Array<{ __id: string; __version: number } & Record<string, unknown>>
+          >(selectSql);
+      } catch (error) {
+        this.handleRawQueryError(error, selectSql);
+      }
     }
 
     const returningWithAutoNumber = Array.from(
@@ -122,8 +128,29 @@ export class RecordComputedUpdateService {
       returningDbFieldNames: returningWithAutoNumber,
     });
     this.logger.debug('updateFromSelect SQL:', sql);
-    return await this.prismaService
-      .txClient()
-      .$queryRawUnsafe<Array<{ __id: string; __version: number } & Record<string, unknown>>>(sql);
+    try {
+      return await this.prismaService
+        .txClient()
+        .$queryRawUnsafe<Array<{ __id: string; __version: number } & Record<string, unknown>>>(sql);
+    } catch (error) {
+      this.handleRawQueryError(error, sql);
+    }
+  }
+
+  private handleRawQueryError(error: unknown, sql: string): never {
+    if (error instanceof Prisma.PrismaClientKnownRequestError) {
+      error.message = `${error.message}\nSQL: ${sql}`;
+      Object.assign(error, { sql });
+      this.logger.error(
+        `updateFromSelect known request error. SQL: ${sql}`,
+        error.stack ?? undefined
+      );
+      throw error;
+    }
+    this.logger.error(
+      `updateFromSelect unexpected query error. SQL: ${sql}`,
+      (error as Error)?.stack
+    );
+    throw error;
   }
 }

apps/nestjs-backend/src/features/record/open-api/record-open-api.service.ts

@@ -128,7 +128,9 @@ export class RecordOpenApiService {
       tableId,
       [recordId],
       undefined,
-      updateRecordRo.fieldKeyType || FieldKeyType.Name
+      updateRecordRo.fieldKeyType || FieldKeyType.Name,
+      undefined,
+      true
     );
 
     if (snapshots.length !== 1) {

apps/nestjs-backend/src/features/record/query-builder/formula-support-generated-column-validator.ts

@@ -1,3 +1,4 @@
+/* eslint-disable sonarjs/no-identical-functions */
 import type {
   TableDomain,
   IFunctionCallInfo,
@@ -59,6 +60,14 @@ export class FormulaSupportGeneratedColumnValidator {
         return false;
       }
 
+      if (this.hasLogicalNonBooleanArgs(tree)) {
+        return false;
+      }
+
+      if (this.containsLogicalFunctions(tree)) {
+        return false;
+      }
+
       // Extract all function calls from the AST
       const collector = new FunctionCallCollectorVisitor();
       const functionCalls = collector.visit(tree);
@@ -137,10 +146,16 @@ export class FormulaSupportGeneratedColumnValidator {
 
     // If it's a formula field, recursively check its dependencies
     if (field.type === FieldType.Formula) {
+      const formulaField = field as FormulaFieldCore;
+
+      if (!formulaField.getIsPersistedAsGeneratedColumn()) {
+        return false;
+      }
+
       visitedFields.add(fieldId);
 
       try {
-        const expression = (field as FormulaFieldCore).getExpression();
+        const expression = formulaField.getExpression();
         if (expression) {
           const tree = parseFormula(expression);
           return this.validateFieldReferences(tree, visitedFields);
@@ -554,12 +569,13 @@ export class FormulaSupportGeneratedColumnValidator {
 
       // eslint-disable-next-line sonarjs/no-identical-functions
       visitChildren(node: RuleNode): boolean {
-        const n = node.childCount;
-        for (let i = 0; i < n; i++) {
-          const child = node.getChild(i);
+        let index = 0;
+        while (index < node.childCount) {
+          const child = node.getChild(index);
           if (child && child.accept(this)) {
             return true;
           }
+          index++;
         }
         return false;
       }
@@ -582,6 +598,89 @@ export class FormulaSupportGeneratedColumnValidator {
     return tree.accept(new DatetimeConcatDetector()) ?? false;
   }
 
+  private hasLogicalNonBooleanArgs(tree: ExprContext): boolean {
+    // eslint-disable-next-line @typescript-eslint/no-this-alias
+    const self = this;
+    class LogicalArgumentDetector extends AbstractParseTreeVisitor<boolean> {
+      protected defaultResult(): boolean {
+        return false;
+      }
+
+      visitChildren(node: RuleNode): boolean {
+        const n = node.childCount;
+        for (let i = 0; i < n; i++) {
+          const child = node.getChild(i);
+          if (child && child.accept(this)) {
+            return true;
+          }
+        }
+        return false;
+      }
+
+      visitFunctionCall(ctx: FunctionCallContext): boolean {
+        const rawName = ctx.func_name().text.toUpperCase();
+        const fnName = normalizeFunctionNameAlias(rawName) as FunctionName;
+        const isLogical =
+          fnName === FunctionName.And ||
+          fnName === FunctionName.Or ||
+          fnName === FunctionName.Not ||
+          fnName === FunctionName.Xor;
+
+        if (isLogical) {
+          const exprs = ctx.expr();
+          for (const exprCtx of exprs) {
+            const argType = self.inferBasicType(exprCtx);
+            if (argType === 'string' || argType === 'number' || argType === 'datetime') {
+              return true;
+            }
+          }
+        }
+
+        return this.visitChildren(ctx);
+      }
+    }
+
+    return tree.accept(new LogicalArgumentDetector()) ?? false;
+  }
+
+  private containsLogicalFunctions(tree: ExprContext): boolean {
+    class LogicalFunctionDetector extends AbstractParseTreeVisitor<boolean> {
+      protected defaultResult(): boolean {
+        return false;
+      }
+
+      visitChildren(node: RuleNode): boolean {
+        let index = 0;
+        while (index < node.childCount) {
+          const child = node.getChild(index);
+          if (child && child.accept(this)) {
+            return true;
+          }
+          index++;
+        }
+        return false;
+      }
+
+      visitFunctionCall(ctx: FunctionCallContext): boolean {
+        const rawName = ctx.func_name().text.toUpperCase();
+        const fnName = normalizeFunctionNameAlias(rawName) as FunctionName;
+        const isLogical =
+          fnName === FunctionName.And ||
+          fnName === FunctionName.Or ||
+          fnName === FunctionName.Not ||
+          fnName === FunctionName.Xor;
+
+        if (isLogical) {
+          return true;
+        }
+
+        return this.visitChildren(ctx);
+      }
+    }
+
+    return tree.accept(new LogicalFunctionDetector()) ?? false;
+  }
+
   // eslint-disable-next-line sonarjs/cognitive-complexity
   private inferBasicType(
     ctx: ExprContext

apps/nestjs-backend/src/features/record/query-builder/providers/pg-record-query-dialect.ts

@@ -246,6 +246,37 @@ export class PgRecordQueryDialect implements IRecordQueryDialectProvider {
     }
   }
 
+  private sanitizeNumericTextExpression(expression: string): string {
+    const textExpr = `((${expression})::text COLLATE "C")`;
+    const sanitized = `REGEXP_REPLACE(${textExpr}, '[^0-9.+-]', '', 'g')`;
+    return `NULLIF(${sanitized}, '' COLLATE "C")::double precision`;
+  }
+
+  private buildJsonNumericSumExpression(fieldExpression: string): string {
+    const expr = `(${fieldExpression})`;
+    const scalarValue = this.sanitizeNumericTextExpression(expr);
+    const arraySum = `(SELECT SUM(${this.sanitizeNumericTextExpression('elem.value')})
+        FROM jsonb_array_elements_text(${expr}::jsonb) AS elem(value))`;
+    return `(CASE
+      WHEN ${expr} IS NULL THEN 0
+      WHEN jsonb_typeof(${expr}::jsonb) = 'array' THEN COALESCE(${arraySum}, 0)
+      ELSE COALESCE(${scalarValue}, 0)
+    END)`;
+  }
+
+  private buildJsonNumericCountExpression(fieldExpression: string): string {
+    const expr = `(${fieldExpression})`;
+    const scalarValue = this.sanitizeNumericTextExpression(expr);
+    const scalarCount = `(CASE WHEN ${scalarValue} IS NULL THEN 0 ELSE 1 END)`;
+    const elementCount = `(SELECT SUM(CASE WHEN ${this.sanitizeNumericTextExpression('elem.value')} IS NULL THEN 0 ELSE 1 END)
+        FROM jsonb_array_elements_text(${expr}::jsonb) AS elem(value))`;
+    return `(CASE
+      WHEN ${expr} IS NULL THEN 0
+      WHEN jsonb_typeof(${expr}::jsonb) = 'array' THEN COALESCE(${elementCount}, 0)
+      ELSE ${scalarCount}
+    END)`;
+  }
+
   private castAgg(sql: string): string {
     // normalize to double precision for numeric rollups
     return `CAST(${sql} AS DOUBLE PRECISION)`;
@@ -263,26 +294,34 @@ export class PgRecordQueryDialect implements IRecordQueryDialectProvider {
     }
   ): string {
     const { targetField, orderByField, rowPresenceExpr, flattenNestedArray } = opts;
+    const isNumericTarget =
+      targetField?.type === FieldType.Number ||
+      (targetField as unknown as { cellValueType?: CellValueType })?.cellValueType ===
+        CellValueType.Number;
+
     switch (fn) {
       case 'sum':
         // Prefer numeric targets: number field or formula resolving to number
-        if (
-          targetField?.type === FieldType.Number ||
-          // Some computed/lookup/rollup/ formula fields expose numeric cellValueType
-          // Use optional chaining to avoid issues on core field types without this prop
-          (targetField as unknown as { cellValueType?: CellValueType })?.cellValueType ===
-            CellValueType.Number
-        ) {
+        if (isNumericTarget) {
+          if (targetField?.isMultipleCellValue) {
+            const numericExpr = this.buildJsonNumericSumExpression(fieldExpression);
+            return this.castAgg(`COALESCE(SUM(${numericExpr}), 0)`);
+          }
           return this.castAgg(`COALESCE(SUM(${fieldExpression}), 0)`);
         }
         // Non-numeric target: avoid SUM() casting errors
         return this.castAgg('SUM(0)');
       case 'average':
-        if (
-          targetField?.type === FieldType.Number ||
-          (targetField as unknown as { cellValueType?: CellValueType })?.cellValueType ===
-            CellValueType.Number
-        ) {
+        if (isNumericTarget) {
+          if (targetField?.isMultipleCellValue) {
+            const sumExpr = this.buildJsonNumericSumExpression(fieldExpression);
+            const countExpr = this.buildJsonNumericCountExpression(fieldExpression);
+            const sumAgg = `COALESCE(SUM(${sumExpr}), 0)`;
+            const countAgg = `COALESCE(SUM(${countExpr}), 0)`;
+            return this.castAgg(
+              `CASE WHEN ${countAgg} = 0 THEN 0 ELSE ${sumAgg} / ${countAgg} END`
+            );
+          }
           return this.castAgg(`COALESCE(AVG(${fieldExpression}), 0)`);
         }
         return this.castAgg('AVG(0)');

apps/nestjs-backend/src/features/record/record-modify/record-create.service.ts

@@ -95,7 +95,7 @@ export class RecordCreateService {
       this.recordService.convertProjection(projection),
       fieldKeyType,
       CellFormat.Json,
-      false
+      true
     );
     return { records: snapshots.map((s) => s.data) };
   }