Merge pull request #4776 from nocodb/fix/formula-hook

fix(nocodb-sdk): formula curly hook logic + formula refactoring

packages/nc-gui/components/virtual-cell/Formula.vue

@@ -24,11 +24,10 @@ const { showEditNonEditableFieldWarning, showClearNonEditableFieldWarning, activ
       <template #title>
         <span class="font-bold">{{ column.colOptions.error }}</span>
       </template>
-
       <span>ERR!</span>
     </a-tooltip>
 
-    <div class="p-2" @dblclick="activateShowEditNonEditableFieldWarning">
+    <div v-else class="p-2" @dblclick="activateShowEditNonEditableFieldWarning">
       <div v-if="urls" v-html="urls" />
 
       <div v-else>{{ result }}</div>

packages/nocodb-sdk/src/lib/formulaHelpers.ts

@@ -8,21 +8,28 @@ export const jsepCurlyHook = {
     jsep.hooks.add('gobble-token', function gobbleCurlyLiteral(env) {
       const OCURLY_CODE = 123; // {
       const CCURLY_CODE = 125; // }
+      let start = -1;
       const { context } = env;
       if (
         !jsep.isIdentifierStart(context.code) &&
         context.code === OCURLY_CODE
       ) {
+        if (start == -1) {
+          start = context.index;
+        }
         context.index += 1;
-        const nodes = context.gobbleExpressions(CCURLY_CODE);
+        context.gobbleExpressions(CCURLY_CODE);
         if (context.code === CCURLY_CODE) {
           context.index += 1;
           env.node = {
             type: jsep.IDENTIFIER,
-            // column name with space would break it down to jsep.IDENTIFIER + jsep.LITERAL
+            name: /{{(.*?)}}/.test(context.expr)
-            // either take node.name for jsep.IDENTIFIER
+              ? // start would be the position of the first curly bracket
-            // or take node.value for jsep.LITERAL
+                // add 2 to point to the first character for expressions like {{col1}}
-            name: nodes.map((node) => node.name || node.value).join(' '),
+                context.expr.slice(start + 2, context.index - 1)
+              : // start would be the position of the first curly bracket
+                // add 1 to point to the first character for expressions like {col1}
+                context.expr.slice(start + 1, context.index - 1),
           };
           return env.node;
         } else {

packages/nocodb/src/lib/db/sql-data-mapper/lib/sql/BaseModelSqlv2.ts

@@ -1206,14 +1206,14 @@ class BaseModelSqlv2 {
   private async getSelectQueryBuilderForFormula(column: Column<any>) {
     const formula = await column.getColOptions<FormulaColumn>();
     if (formula.error) throw new Error(`Formula error: ${formula.error}`);
-    const selectQb = await formulaQueryBuilderv2(
+    const qb = await formulaQueryBuilderv2(
       formula.formula,
       null,
       this.dbDriver,
-      this.model
+      this.model,
+      column
     );
-
+    return qb;
-    return selectQb;
   }
 
   async getProto() {
@@ -1502,7 +1502,6 @@ class BaseModelSqlv2 {
               const selectQb = await this.getSelectQueryBuilderForFormula(
                 column
               );
-              // todo:  verify syntax of as ? / ??
               qb.select(
                 this.dbDriver.raw(`?? as ??`, [
                   selectQb.builder,
@@ -1510,7 +1509,10 @@ class BaseModelSqlv2 {
                 ])
               );
             } catch {
-              continue;
+              // return dummy select
+              qb.select(
+                this.dbDriver.raw(`'ERR' as ??`, [sanitize(column.title)])
+              );
             }
           }
           break;

packages/nocodb/src/lib/db/sql-data-mapper/lib/sql/conditionV2.ts

@@ -246,7 +246,7 @@ const parseConditionV2 = async (
       const model = await column.getModel();
       const formula = await column.getColOptions<FormulaColumn>();
       const builder = (
-        await formulaQueryBuilderv2(formula.formula, null, knex, model)
+        await formulaQueryBuilderv2(formula.formula, null, knex, model, column)
       ).builder;
       return parseConditionV2(
         new Filter({ ...filter, value: knex.raw('?', [filter.value]) } as any),

packages/nocodb/src/lib/db/sql-data-mapper/lib/sql/formulav2/formulaQueryBuilderv2.ts

@@ -1,6 +1,7 @@
 import jsep from 'jsep';
 import mapFunctionName from '../mapFunctionName';
 import Model from '../../../../../models/Model';
+import Column from '../../../../../models/Column';
 import genRollupSelectv2 from '../genRollupSelectv2';
 import RollupColumn from '../../../../../models/RollupColumn';
 import FormulaColumn from '../../../../../models/FormulaColumn';
@@ -9,6 +10,8 @@ import LinkToAnotherRecordColumn from '../../../../../models/LinkToAnotherRecord
 import LookupColumn from '../../../../../models/LookupColumn';
 import { jsepCurlyHook, UITypes } from 'nocodb-sdk';
 import { validateDateWithUnknownFormat } from '../helpers/formulaFnHelper';
+import { CacheGetType, CacheScope } from '../../../../../utils/globals';
+import NocoCache from '../../../../../cache/NocoCache';
 
 // todo: switch function based on database
 
@@ -45,16 +48,16 @@ const getAggregateFn: (fnName: string) => (args: { qb; knex?; cn }) => any = (
   }
 };
 
-export default async function formulaQueryBuilderv2(
+async function _formulaQueryBuilder(
   _tree,
   alias,
   knex: XKnex,
   model: Model,
   aliasToColumn = {}
 ) {
-  // register jsep curly hook
+  // formula may include double curly brackets in previous version
-  jsep.plugins.register(jsepCurlyHook);
+  // convert to single curly bracket here for compatibility
-  const tree = jsep(_tree);
+  const tree = jsep(_tree.replaceAll('{{', '{').replaceAll('}}', '}'));
 
   const columnIdToUidt = {};
 
@@ -66,7 +69,7 @@ export default async function formulaQueryBuilderv2(
       case UITypes.Formula:
         {
           const formulOption = await col.getColOptions<FormulaColumn>();
-          const { builder } = await formulaQueryBuilderv2(
+          const { builder } = await _formulaQueryBuilder(
             formulOption.formula,
             alias,
             knex,
@@ -340,7 +343,7 @@ export default async function formulaQueryBuilderv2(
                   const formulaOption =
                     await lookupColumn.getColOptions<FormulaColumn>();
                   const lookupModel = await lookupColumn.getModel();
-                  const { builder } = await formulaQueryBuilderv2(
+                  const { builder } = await _formulaQueryBuilder(
                     formulaOption.formula,
                     '',
                     knex,
@@ -771,3 +774,74 @@ export default async function formulaQueryBuilderv2(
   };
   return { builder: fn(tree, alias) };
 }
+
+function getTnPath(tb: Model, knex) {
+  const schema = knex.searchPath?.();
+  if (knex.clientType() === 'mssql' && schema) {
+    return knex.raw('??.??', [schema, tb.table_name]);
+  } else if (knex.clientType() === 'snowflake') {
+    return [
+      knex.client.config.connection.database,
+      knex.client.config.connection.schema,
+      tb.table_name,
+    ].join('.');
+  } else {
+    return tb.table_name;
+  }
+}
+
+export default async function formulaQueryBuilderv2(
+  _tree,
+  alias,
+  knex: XKnex,
+  model: Model,
+  column?: Column,
+  aliasToColumn = {}
+) {
+  // register jsep curly hook once only
+  jsep.plugins.register(jsepCurlyHook);
+  // generate qb
+  const qb = await _formulaQueryBuilder(
+    _tree,
+    alias,
+    knex,
+    model,
+    aliasToColumn
+  );
+
+  try {
+    // dry run qb.builder to see if it will break the grid view or not
+    // if so, set formula error and show empty selectQb instead
+    await knex(getTnPath(model, knex)).select(qb.builder).as('dry-run-only');
+
+    // if column is provided, i.e. formula has been created
+    if (column) {
+      const formula = await column.getColOptions<FormulaColumn>();
+      // clean the previous formula error if the formula works this time
+      if (formula.error) {
+        await FormulaColumn.update(formula.id, {
+          error: null,
+        });
+      }
+    }
+  } catch (e) {
+    console.error(e);
+    if (column) {
+      const formula = await column.getColOptions<FormulaColumn>();
+      // add formula error to show in UI
+      await FormulaColumn.update(formula.id, {
+        error: e.message,
+      });
+      // update cache to reflect the error in UI
+      const key = `${CacheScope.COL_FORMULA}:${column.id}`;
+      let o = await NocoCache.get(key, CacheGetType.TYPE_OBJECT);
+      if (o) {
+        o = { ...o, error: e.message };
+        // set cache
+        await NocoCache.set(key, o);
+      }
+    }
+    throw new Error(`Formula error: ${e.message}`);
+  }
+  return qb;
+}

packages/nocodb/src/lib/db/sql-data-mapper/lib/sql/sortV2.ts

@@ -51,7 +51,8 @@ export default async function sortV2(
               ).formula,
               null,
               knex,
-              model
+              model,
+              column
             )
           ).builder;
           qb.orderBy(builder, sort.direction || 'asc');
@@ -161,7 +162,8 @@ export default async function sortV2(
                       ).formula,
                       null,
                       knex,
-                      model
+                      model,
+                      column
                     )
                   ).builder;
 

packages/nocodb/src/lib/meta/api/columnApis.ts

@@ -40,6 +40,7 @@ import { metaApiMetrics } from '../helpers/apiMetrics';
 import FormulaColumn from '../../models/FormulaColumn';
 import KanbanView from '../../models/KanbanView';
 import { MetaTable } from '../../utils/globals';
+import formulaQueryBuilderv2 from '../../db/sql-data-mapper/lib/sql/formulav2/formulaQueryBuilderv2';
 
 const randomID = customAlphabet('1234567890abcdefghijklmnopqrstuvwxyz_', 10);
 
@@ -523,6 +524,16 @@ export async function columnAdd(
         colBody.formula_raw || colBody.formula,
         table.columns
       );
+
+      try {
+        // test the query to see if it is valid in db level
+        const dbDriver = NcConnectionMgrv2.get(base);
+        await formulaQueryBuilderv2(colBody.formula, null, dbDriver, table);
+      } catch (e) {
+        console.error(e);
+        NcError.badRequest('Invalid Formula');
+      }
+
       await Column.insert({
         ...colBody,
         fk_model_id: table.id,
@@ -759,6 +770,16 @@ export async function columnUpdate(req: Request, res: Response<TableType>) {
           colBody.formula_raw || colBody.formula,
           table.columns
         );
+
+        try {
+          // test the query to see if it is valid in db level
+          const dbDriver = NcConnectionMgrv2.get(base);
+          await formulaQueryBuilderv2(colBody.formula, null, dbDriver, table);
+        } catch (e) {
+          console.error(e);
+          NcError.badRequest('Invalid Formula');
+        }
+
         await Column.update(column.id, {
           // title: colBody.title,
           ...column,