From ec150b1a5a76e7239a5466cc2db4bce26ae20206 Mon Sep 17 00:00:00 2001 From: Pranav C Date: Thu, 30 Mar 2023 18:31:59 +0530 Subject: [PATCH] fix(nocodb): corrections Signed-off-by: Pranav C --- .../sql/formulav2/formulaQueryBuilderv2.ts | 116 ++++++++++---- .../lib/sql/functionMappings/commonFns.ts | 102 ++++++------ .../lib/sql/functionMappings/pg.ts | 146 ++++++++++-------- 3 files changed, 232 insertions(+), 132 deletions(-) diff --git a/packages/nocodb/src/lib/db/sql-data-mapper/lib/sql/formulav2/formulaQueryBuilderv2.ts b/packages/nocodb/src/lib/db/sql-data-mapper/lib/sql/formulav2/formulaQueryBuilderv2.ts index 3bd4a073d8..a3910464ef 100644 --- a/packages/nocodb/src/lib/db/sql-data-mapper/lib/sql/formulav2/formulaQueryBuilderv2.ts +++ b/packages/nocodb/src/lib/db/sql-data-mapper/lib/sql/formulav2/formulaQueryBuilderv2.ts @@ -628,36 +628,35 @@ async function _formulaQueryBuilder( break; } - return knex.raw( - `${pt.callee.name}(${pt.arguments - .map((arg) => { - const query = fn(arg).toQuery(); - if (pt.callee.name === 'CONCAT') { - if (knex.clientType() === 'mysql2') { - // mysql2: CONCAT() returns NULL if any argument is NULL. - // adding IFNULL to convert NULL values to empty strings - return `IFNULL(${query}, '')`; - } else { - // do nothing - // pg / mssql: Concatenate all arguments. NULL arguments are ignored. - // sqlite3: special handling - See BinaryExpression + return { + builder: knex.raw( + `${pt.callee.name}(${pt.arguments + .map((arg) => { + const query = fn(arg).toQuery(); + if (pt.callee.name === 'CONCAT') { + if (knex.clientType() === 'mysql2') { + // mysql2: CONCAT() returns NULL if any argument is NULL. + // adding IFNULL to convert NULL values to empty strings + return `IFNULL(${query}, '')`; + } else { + // do nothing + // pg / mssql: Concatenate all arguments. NULL arguments are ignored. + // sqlite3: special handling - See BinaryExpression + } } - } - return query; - }) - .join()})${colAlias}`.replace(/\?/g, '\\?') - ); + return query; + }) + .join()})${colAlias}`.replace(/\?/g, '\\?') + ), + }; } else if (pt.type === 'Literal') { - return knex.raw(`?${colAlias}`, [pt.value]); + return { builder: knex.raw(`?${colAlias}`, [pt.value]) }; } else if (pt.type === 'Identifier') { - const { builder } = await aliasToColumn?.[pt.name]() + const { builder } = await aliasToColumn?.[pt.name]?.(); if (typeof builder === 'function') { - return knex.raw( - `??${colAlias}`, - await builder(pt.fnName) - ); + return { builder: knex.raw(`??${colAlias}`, await builder(pt.fnName)) }; } - return knex.raw(`??${colAlias}`, [aliasToColumn?.[pt.name] || pt.name]); + return { builder: knex.raw(`??${colAlias}`, [builder || pt.name]) }; } else if (pt.type === 'BinaryExpression') { if (pt.operator === '==') { pt.operator = '='; @@ -678,8 +677,8 @@ async function _formulaQueryBuilder( pt.left.fnName = pt.left.fnName || 'ARITH'; pt.right.fnName = pt.right.fnName || 'ARITH'; - const left = fn(pt.left, null, pt.operator).toQuery(); - const right = fn(pt.right, null, pt.operator).toQuery(); + const left = (await fn(pt.left, null, pt.operator)).builder.toQuery(); + const right = (await fn(pt.right, null, pt.operator)).builder.toQuery(); let sql = `${left} ${pt.operator} ${right}${colAlias}`; // comparing a date with empty string would throw @@ -788,7 +787,9 @@ async function _formulaQueryBuilder( return { builder: query }; } }; - return { builder: fn(tree, alias) }; + const builder = (await fn(tree, alias)).builder; + + return { builder }; } function getTnPath(tb: Model, knex, tableAlias?: string) { @@ -877,3 +878,62 @@ export default async function formulaQueryBuilderv2( } return qb; } + +export async function validateFormula( + _tree, + alias, + knex: XKnex, + model: Model, + column?: Column, + aliasToColumn = {}, + tableAlias?: string +) { + // register jsep curly hook once only + jsep.plugins.register(jsepCurlyHook); + // generate qb + const qb = await _formulaQueryBuilder( + _tree, + alias, + knex, + model, + aliasToColumn, + tableAlias + ); + + 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, tableAlias)) + .select(qb.builder) + .as('dry-run-only'); + + // if column is provided, i.e. formula has been created + if (column) { + const formula = await column.getColOptions(); + // 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(); + // 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}`); + } +} diff --git a/packages/nocodb/src/lib/db/sql-data-mapper/lib/sql/functionMappings/commonFns.ts b/packages/nocodb/src/lib/db/sql-data-mapper/lib/sql/functionMappings/commonFns.ts index 1fac4fbbd7..9c3a2f3033 100644 --- a/packages/nocodb/src/lib/db/sql-data-mapper/lib/sql/functionMappings/commonFns.ts +++ b/packages/nocodb/src/lib/db/sql-data-mapper/lib/sql/functionMappings/commonFns.ts @@ -28,7 +28,11 @@ export default { ) .toQuery(); } - return args.knex.raw(`CASE ${switchVal} ${query}\n END${args.colAlias}`); + return { + builder: args.knex.raw( + `CASE ${switchVal} ${query}\n END${args.colAlias}` + ), + }; }, IF: async (args: MapFnArgs) => { let query = args.knex @@ -42,62 +46,74 @@ export default { .toQuery(); if (args.pt.arguments[2]) { query += args.knex - .raw(`\n\tELSE ${(await args.fn(args.pt.arguments[2])).builder.toQuery()}`) + .raw( + `\n\tELSE ${(await args.fn(args.pt.arguments[2])).builder.toQuery()}` + ) .toQuery(); } - return args.knex.raw(`CASE ${query}\n END${args.colAlias}`); + return { builder: args.knex.raw(`CASE ${query}\n END${args.colAlias}`) }; }, - TRUE: (_args) => 1, - FALSE: (_args) => 0, + TRUE: 1, + FALSE: 0, AND: async (args: MapFnArgs) => { - return args.knex.raw( - `${args.knex - .raw( - `${( - await Promise.all( - args.pt.arguments.map(async (ar) => - (await args.fn(ar)).builder.toQuery() + return { + builder: args.knex.raw( + `${args.knex + .raw( + `${( + await Promise.all( + args.pt.arguments.map(async (ar) => + (await args.fn(ar)).builder.toQuery() + ) ) - ) - ).join(' AND ')}` - ) - .wrap('(', ')') - .toQuery()}${args.colAlias}` - ); + ).join(' AND ')}` + ) + .wrap('(', ')') + .toQuery()}${args.colAlias}` + ), + }; }, OR: async (args: MapFnArgs) => { - return args.knex.raw( - `${args.knex - .raw( - `${( - await Promise.all( - args.pt.arguments.map(async (ar) => - (await args.fn(ar)).builder.toQuery() + return { + builder: args.knex.raw( + `${args.knex + .raw( + `${( + await Promise.all( + args.pt.arguments.map(async (ar) => + (await args.fn(ar)).builder.toQuery() + ) ) - ) - ).join(' OR ')}` - ) - .wrap('(', ')') - .toQuery()}${args.colAlias}` - ); + ).join(' OR ')}` + ) + .wrap('(', ')') + .toQuery()}${args.colAlias}` + ), + }; }, AVG: (args: MapFnArgs) => { if (args.pt.arguments.length > 1) { - return args.fn( - { - type: 'BinaryExpression', - operator: '/', - left: { ...args.pt, callee: { name: 'SUM' } }, - right: { type: 'Literal', value: args.pt.arguments.length }, - }, - args.a, - args.prevBinaryOp - ); + return { + builder: args.fn( + { + type: 'BinaryExpression', + operator: '/', + left: { ...args.pt, callee: { name: 'SUM' } }, + right: { type: 'Literal', value: args.pt.arguments.length }, + }, + args.a, + args.prevBinaryOp + ), + }; } else { - return args.fn(args.pt.arguments[0], args.a, args.prevBinaryOp); + return { + builder: args.fn(args.pt.arguments[0], args.a, args.prevBinaryOp), + }; } }, FLOAT: async (args: MapFnArgs) => { - return (await args.fn(args.pt?.arguments?.[0])).builder.wrap('(', ')'); + return { + builder: (await args.fn(args.pt?.arguments?.[0])).builder.wrap('(', ')'), + }; }, }; diff --git a/packages/nocodb/src/lib/db/sql-data-mapper/lib/sql/functionMappings/pg.ts b/packages/nocodb/src/lib/db/sql-data-mapper/lib/sql/functionMappings/pg.ts index b361996859..a03bc664e4 100644 --- a/packages/nocodb/src/lib/db/sql-data-mapper/lib/sql/functionMappings/pg.ts +++ b/packages/nocodb/src/lib/db/sql-data-mapper/lib/sql/functionMappings/pg.ts @@ -13,43 +13,53 @@ const pg = { POWER: 'pow', SQRT: 'sqrt', SEARCH: async (args: MapFnArgs) => { - return args.knex.raw( - `POSITION(${args.knex.raw( - (await args.fn(args.pt.arguments[1])).builder.toQuery() - )} in ${args.knex - .raw((await args.fn(args.pt.arguments[0])).builder) - .toQuery()})${args.colAlias}` - ); + return { + builder: args.knex.raw( + `POSITION(${args.knex.raw( + (await args.fn(args.pt.arguments[1])).builder.toQuery() + )} in ${args.knex + .raw((await args.fn(args.pt.arguments[0])).builder) + .toQuery()})${args.colAlias}` + ), + }; }, INT(args: MapFnArgs) { // todo: correction - return args.knex.raw( - `REGEXP_REPLACE(COALESCE(${args.fn( - args.pt.arguments[0] - )}::character varying, '0'), '[^0-9]+|\\.[0-9]+' ,'')${args.colAlias}` - ); + return { + builder: args.knex.raw( + `REGEXP_REPLACE(COALESCE(${args.fn( + args.pt.arguments[0] + )}::character varying, '0'), '[^0-9]+|\\.[0-9]+' ,'')${args.colAlias}` + ), + }; }, MID: 'SUBSTR', FLOAT: ({ fn, knex, pt, colAlias }: MapFnArgs) => { - return knex - .raw(`CAST(${fn(pt.arguments[0])} as DOUBLE PRECISION)${colAlias}`) - .wrap('(', ')'); + return { + builder: knex + .raw(`CAST(${fn(pt.arguments[0])} as DOUBLE PRECISION)${colAlias}`) + .wrap('(', ')'), + }; }, ROUND: ({ fn, knex, pt, colAlias }: MapFnArgs) => { - return knex.raw( - `ROUND((${fn(pt.arguments[0])})::numeric, ${ - pt?.arguments[1] ? fn(pt.arguments[1]) : 0 - }) ${colAlias}` - ); + return { + builder: knex.raw( + `ROUND((${fn(pt.arguments[0])})::numeric, ${ + pt?.arguments[1] ? fn(pt.arguments[1]) : 0 + }) ${colAlias}` + ), + }; }, DATEADD: ({ fn, knex, pt, colAlias }: MapFnArgs) => { - return knex.raw( - `${fn(pt.arguments[0])} + (${fn(pt.arguments[1])} || + return { + builder: knex.raw( + `${fn(pt.arguments[0])} + (${fn(pt.arguments[1])} || '${String(fn(pt.arguments[2])).replace( /["']/g, '' )}')::interval${colAlias}` - ); + ), + }; }, DATETIME_DIFF: async ({ fn, knex, pt, colAlias }: MapFnArgs) => { const datetime_expr1 = fn(pt.arguments[0]); @@ -99,61 +109,75 @@ const pg = { default: sql = ''; } - return knex.raw(`${sql} ${colAlias}`); + return { builder: knex.raw(`${sql} ${colAlias}`) }; }, WEEKDAY: async ({ fn, knex, pt, colAlias }: MapFnArgs) => { // isodow: the day of the week as Monday (1) to Sunday (7) // WEEKDAY() returns an index from 0 to 6 for Monday to Sunday - return knex.raw( - `(EXTRACT(ISODOW FROM ${ - pt.arguments[0].type === 'Literal' - ? `date '${dayjs((await fn(pt.arguments[0])).builder).format( - 'YYYY-MM-DD' - )}'` - : fn(pt.arguments[0]) - }) - 1 - ${getWeekdayByText( - pt?.arguments[1]?.value - )} % 7 + 7) ::INTEGER % 7 ${colAlias}` - ); + return { + builder: knex.raw( + `(EXTRACT(ISODOW FROM ${ + pt.arguments[0].type === 'Literal' + ? `date '${dayjs((await fn(pt.arguments[0])).builder).format( + 'YYYY-MM-DD' + )}'` + : fn(pt.arguments[0]) + }) - 1 - ${getWeekdayByText( + pt?.arguments[1]?.value + )} % 7 + 7) ::INTEGER % 7 ${colAlias}` + ), + }; }, AND: async (args: MapFnArgs) => { - return args.knex.raw( - `CASE WHEN ${args.knex - .raw( - `${args.pt.arguments - .map(async (ar) => (await args.fn(ar, '', 'AND')).builder.toQuery()) - .join(' AND ')}` - ) - .wrap('(', ')') - .toQuery()} THEN TRUE ELSE FALSE END ${args.colAlias}` - ); + return { + builder: args.knex.raw( + `CASE WHEN ${args.knex + .raw( + `${args.pt.arguments + .map(async (ar) => + (await args.fn(ar, '', 'AND')).builder.toQuery() + ) + .join(' AND ')}` + ) + .wrap('(', ')') + .toQuery()} THEN TRUE ELSE FALSE END ${args.colAlias}` + ), + }; }, OR: async (args: MapFnArgs) => { - return args.knex.raw( - `CASE WHEN ${args.knex - .raw( - `${args.pt.arguments - .map(async (ar) => (await args.fn(ar, '', 'OR')).builder.toQuery()) - .join(' OR ')}` - ) - .wrap('(', ')') - .toQuery()} THEN TRUE ELSE FALSE END ${args.colAlias}` - ); + return { + builder: args.knex.raw( + `CASE WHEN ${args.knex + .raw( + `${args.pt.arguments + .map(async (ar) => + (await args.fn(ar, '', 'OR')).builder.toQuery() + ) + .join(' OR ')}` + ) + .wrap('(', ')') + .toQuery()} THEN TRUE ELSE FALSE END ${args.colAlias}` + ), + }; }, SUBSTR: ({ fn, knex, pt, colAlias }: MapFnArgs) => { const str = fn(pt.arguments[0]); const positionFrom = fn(pt.arguments[1] ?? 1); const numberOfCharacters = fn(pt.arguments[2] ?? ''); - return knex.raw( - `SUBSTR(${str}::TEXT, ${positionFrom}${ - numberOfCharacters ? ', ' + numberOfCharacters : '' - })${colAlias}` - ); + return { + builder: knex.raw( + `SUBSTR(${str}::TEXT, ${positionFrom}${ + numberOfCharacters ? ', ' + numberOfCharacters : '' + })${colAlias}` + ), + }; }, MOD: ({ fn, knex, pt, colAlias }: MapFnArgs) => { const x = fn(pt.arguments[0]); const y = fn(pt.arguments[1]); - return knex.raw(`MOD((${x})::NUMERIC, (${y})::NUMERIC) ${colAlias}`); + return { + builder: knex.raw(`MOD((${x})::NUMERIC, (${y})::NUMERIC) ${colAlias}`), + }; }, };