Elgato_dark/ToolJet
1
0
Fork 0
mirror of https://github.com/ToolJet/ToolJet synced 2026-05-24 09:28:31 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions

Fix: Validation of aggregate conditions (#10261)

Browse source 
* fix: added validation for empty column names in list rows for aggregate condition

* fix: handle empty conditions for aggregates

* fix: error message for aggregate condition in join operation has been updated

* fix: handled a scenario where table and column name with capital letter was not working properly in aggregate and groupby
This commit is contained in:
Ganesh Kumar 2024-07-05 13:53:23 +05:30 committed by GitHub
parent 063a8d2b85
commit 6296a347e8
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
2 changed files with 66 additions and 52 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

19
server/src/services/tooljet_db.service.ts
View file

@ -12,8 +12,9 @@ import {
import { InjectEntityManager } from '@nestjs/typeorm';
import { InternalTable } from 'src/entities/internal_table.entity';
import { isString, isEmpty, camelCase } from 'lodash';
import { TooljetDatabaseError, TooljetDbActions } from 'src/modules/tooljet_db/tooljet-db.types';
import { PostgrestError, TooljetDatabaseError, TooljetDbActions } from 'src/modules/tooljet_db/tooljet-db.types';
import { v4 as uuidv4 } from 'uuid';
import { QueryError } from '@tooljet/plugins/packages/common';
export type TableColumnSchema = {
column_name: string;
@ -816,8 +817,8 @@ export class TooljetDbService {
const queryBuilder = this.buildJoinQuery(joinQueryJson, internalTableIdToNameMap);
return await queryBuilder.getRawMany();
} catch (error) {
const errorObj = new QueryFailedError(error, [], error);
throw new TooljetDatabaseError(
const errorObj = new QueryFailedError(error, [], new PostgrestError(error));
const tjdbErrorObj = new TooljetDatabaseError(
error.message,
{
origin: 'join_tables',
@ -825,6 +826,8 @@ export class TooljetDbService {
},
errorObj
);
const alteredErrorMessage = tjdbErrorObj.toString();
throw new QueryError(alteredErrorMessage, alteredErrorMessage, {});
}
}
@ -849,12 +852,16 @@ export class TooljetDbService {
if (!isEmpty(queryJson.aggregates)) {
Object.entries(queryJson.aggregates).forEach(([_key, aggregateParams]) => {
const { aggFx, column, table_id: tableId } = aggregateParams as any;
if (isEmpty(column) || isEmpty(aggFx))
throw new Error('There are empty values in certain aggregate conditions.');
const allowedAggFunctions = ['sum', 'count'];
if (!allowedAggFunctions.includes(aggFx)) {
throw new BadRequestException('Invalid aggregate function');
}
queryBuilder.addSelect(
`${AggregateFunctions[aggFx]}(${internalTableIdToNameMap[tableId]}.${column})`,
`${AggregateFunctions[aggFx]}("${internalTableIdToNameMap[tableId]}"."${column}")`,
`${internalTableIdToNameMap[tableId]}_${column}_${aggFx}`
);
});
@ -867,12 +874,12 @@ export class TooljetDbService {
groupByColumList.forEach((groupByColum) => {
// The 'SELECT' statement needs to have 'GROUP_BY' columns added.
queryBuilder.addSelect(
`${internalTableIdToNameMap[groupByTableId]}.${groupByColum}`,
`"${internalTableIdToNameMap[groupByTableId]}"."${groupByColum}"`,
`${internalTableIdToNameMap[groupByTableId]}_${groupByColum}`
);
// Building `GROUP_BY` statement
queryBuilder.addGroupBy(`${internalTableIdToNameMap[groupByTableId]}.${groupByColum}`);
queryBuilder.addGroupBy(`"${internalTableIdToNameMap[groupByTableId]}"."${groupByColum}"`);
});
}
});

99
server/src/services/tooljet_db_operations.service.ts
View file

@ -7,6 +7,7 @@ import { PostgrestProxyService } from './postgrest_proxy.service';
import { maybeSetSubPath } from 'src/helpers/utils.helper';
import { EntityManager } from 'typeorm';
import { InternalTable } from 'src/entities/internal_table.entity';
import { QueryError } from '@tooljet/plugins/packages/common';
@Injectable()
export class TooljetDbOperationsService implements QueryService {
@ -58,59 +59,63 @@ export class TooljetDbOperationsService implements QueryService {
data: {},
};
}
const { table_id: tableId, list_rows: listRows, organization_id: organizationId } = queryOptions;
const query = [];
try {
const { table_id: tableId, list_rows: listRows, organization_id: organizationId } = queryOptions;
const query = [];
if (!isEmpty(listRows)) {
const {
limit,
where_filters: whereFilters,
order_filters: orderFilters,
offset,
aggregates = {},
group_by: groupBy = {},
} = listRows;
if (!isEmpty(listRows)) {
const {
limit,
where_filters: whereFilters,
order_filters: orderFilters,
offset,
aggregates = {},
group_by: groupBy = {},
} = listRows;
const internalTable = await this.manager.findOne(InternalTable, {
where: {
organizationId,
id: tableId,
},
});
const internalTable = await this.manager.findOne(InternalTable, {
where: {
organizationId,
id: tableId,
},
});
if (!internalTable) throw new NotFoundException('Table not found');
if (!internalTable) throw new NotFoundException('Table not found');
if (limit && isNaN(limit)) {
return {
status: 'failed',
errorMessage: 'Limit should be a number.',
data: {},
};
if (limit && isNaN(limit)) {
return {
status: 'failed',
errorMessage: 'Limit should be a number.',
data: {},
};
}
const whereQuery = buildPostgrestQuery(whereFilters);
const orderQuery = buildPostgrestQuery(orderFilters);
if (!isEmpty(aggregates) || !isEmpty(groupBy)) {
const groupByAndAggregateQueryList = this.buildAggregateAndGroupByQuery(
internalTable.tableName,
aggregates,
groupBy
);
if (groupByAndAggregateQueryList.length) query.push(`select=${groupByAndAggregateQueryList.join(',')}`);
}
!isEmpty(whereQuery) && query.push(whereQuery);
!isEmpty(orderQuery) && query.push(orderQuery);
!isEmpty(limit) && query.push(`limit=${limit}`);
!isEmpty(offset) && query.push(`offset=${offset}`);
}
const whereQuery = buildPostgrestQuery(whereFilters);
const orderQuery = buildPostgrestQuery(orderFilters);
if (!isEmpty(aggregates) || !isEmpty(groupBy)) {
const groupByAndAggregateQueryList = this.buildAggregateAndGroupByQuery(
internalTable.tableName,
aggregates,
groupBy
);
if (groupByAndAggregateQueryList.length) query.push(`select=${groupByAndAggregateQueryList.join(',')}`);
}
!isEmpty(whereQuery) && query.push(whereQuery);
!isEmpty(orderQuery) && query.push(orderQuery);
!isEmpty(limit) && query.push(`limit=${limit}`);
!isEmpty(offset) && query.push(`offset=${offset}`);
const headers = { 'data-query-id': queryOptions.id, 'tj-workspace-id': queryOptions.organization_id };
const url =
query.length > 0
? `/api/tooljet-db/proxy/${tableId}` + `?${query.join('&')}`
: `/api/tooljet-db/proxy/${tableId}`;
return await this.proxyPostgrest(maybeSetSubPath(url), 'GET', headers);
} catch (error) {
throw new QueryError(error.message, error.message, {});
}
const headers = { 'data-query-id': queryOptions.id, 'tj-workspace-id': queryOptions.organization_id };
const url =
query.length > 0
? `/api/tooljet-db/proxy/${tableId}` + `?${query.join('&')}`
: `/api/tooljet-db/proxy/${tableId}`;
return await this.proxyPostgrest(maybeSetSubPath(url), 'GET', headers);
}
async createRow(queryOptions): Promise<QueryResult> {
@ -258,6 +263,8 @@ export class TooljetDbOperationsService implements QueryService {
if (!isEmpty(aggregates)) {
Object.entries(aggregates).forEach(([_key, aggregateDetail]) => {
const { aggFx, column } = aggregateDetail;
if (isEmpty(column) || isEmpty(aggFx))
throw new Error('There are empty values in certain aggregate conditions.');
if (aggFx && column) query.push(`${tableName}_${column}_${aggFx}:${column}.${AggregateFunctions[aggFx]}()`);
});
}