Skip to content

Commit

Permalink
fix: json field grouping and multi-value field SUM in base query inte…
Browse files Browse the repository at this point in the history
…rface (#1027)

* fix: json field grouping and multi-value field SUM in base query interface

* fix: base query joinSelect support sqlite
  • Loading branch information
boris-w authored Oct 28, 2024
1 parent e0dfc25 commit 1038cb9
Show file tree
Hide file tree
Showing 9 changed files with 113 additions and 8 deletions.
11 changes: 11 additions & 0 deletions apps/nestjs-backend/src/db-provider/base-query/abstract.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
import type { Knex } from 'knex';

export abstract class BaseQueryAbstract {
constructor(protected readonly knex: Knex) {}

abstract jsonSelect(
queryBuilder: Knex.QueryBuilder,
dbFieldName: string,
alias: string
): Knex.QueryBuilder;
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
import type { Knex } from 'knex';
import { BaseQueryAbstract } from './abstract';

export class BaseQueryPostgres extends BaseQueryAbstract {
constructor(protected readonly knex: Knex) {
super(knex);
}

jsonSelect(
queryBuilder: Knex.QueryBuilder,
dbFieldName: string,
alias: string
): Knex.QueryBuilder {
return queryBuilder.select(this.knex.raw(`MAX(??::text) AS ??`, [dbFieldName, alias]));
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
import type { Knex } from 'knex';
import { BaseQueryAbstract } from './abstract';

export class BaseQuerySqlite extends BaseQueryAbstract {
constructor(protected readonly knex: Knex) {
super(knex);
}

jsonSelect(
queryBuilder: Knex.QueryBuilder,
dbFieldName: string,
alias: string
): Knex.QueryBuilder {
return queryBuilder.select(this.knex.raw(`MAX(??) AS ??`, [dbFieldName, alias]));
}
}
3 changes: 3 additions & 0 deletions apps/nestjs-backend/src/db-provider/db.provider.interface.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import type { Knex } from 'knex';
import type { IFieldInstance } from '../features/field/model/factory';
import type { SchemaType } from '../features/field/util';
import type { IAggregationQueryInterface } from './aggregation-query/aggregation-query.interface';
import type { BaseQueryAbstract } from './base-query/abstract';
import type { IFilterQueryInterface } from './filter-query/filter-query.interface';
import type { IGroupQueryExtra, IGroupQueryInterface } from './group-query/group-query.interface';
import type { ISortQueryInterface } from './sort-query/sort-query.interface';
Expand Down Expand Up @@ -130,4 +131,6 @@ export interface IDbProvider {
dbFieldName: string,
isMultipleCellValue?: boolean | null
): void;

baseQuery(): BaseQueryAbstract;
}
6 changes: 6 additions & 0 deletions apps/nestjs-backend/src/db-provider/postgres.provider.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@ import type { IFieldInstance } from '../features/field/model/factory';
import type { SchemaType } from '../features/field/util';
import type { IAggregationQueryInterface } from './aggregation-query/aggregation-query.interface';
import { AggregationQueryPostgres } from './aggregation-query/postgres/aggregation-query.postgres';
import type { BaseQueryAbstract } from './base-query/abstract';
import { BaseQueryPostgres } from './base-query/base-query.postgres';
import type {
IAggregationQueryExtra,
IDbProvider,
Expand Down Expand Up @@ -327,4 +329,8 @@ export class PostgresProvider implements IDbProvider {
);
}
}

baseQuery(): BaseQueryAbstract {
return new BaseQueryPostgres(this.knex);
}
}
6 changes: 6 additions & 0 deletions apps/nestjs-backend/src/db-provider/sqlite.provider.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@ import type { IFieldInstance } from '../features/field/model/factory';
import type { SchemaType } from '../features/field/util';
import type { IAggregationQueryInterface } from './aggregation-query/aggregation-query.interface';
import { AggregationQuerySqlite } from './aggregation-query/sqlite/aggregation-query.sqlite';
import type { BaseQueryAbstract } from './base-query/abstract';
import { BaseQuerySqlite } from './base-query/base-query.sqlite';
import type {
IAggregationQueryExtra,
IDbProvider,
Expand Down Expand Up @@ -279,4 +281,8 @@ export class SqliteProvider implements IDbProvider {
originQueryBuilder.distinct(this.knex.raw(`json_extract(${dbFieldName}, '$.id') AS user_id`));
}
}

baseQuery(): BaseQueryAbstract {
return new BaseQuerySqlite(this.knex);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,6 @@ export class BaseQueryService {
throw new BadRequestException(`Query failed: ${query}, ${e.message}`);
});
const columns = this.convertFieldMapToColumn(fieldMap);

return {
rows: await this.dbRows2Rows(rows, columns, cellFormat),
columns,
Expand All @@ -132,6 +131,7 @@ export class BaseQueryService {
fieldMap,
queryBuilder,
baseId,
dbTableName,
});
}
const { queryBuilder, fieldMap } = await this.parseBaseQuery(baseId, baseQuery.from, depth + 1);
Expand All @@ -149,6 +149,7 @@ export class BaseQueryService {
),
queryBuilder: this.knex(queryBuilder.as(alias)),
baseId,
dbTableName: alias,
});
}

Expand All @@ -158,9 +159,10 @@ export class BaseQueryService {
baseId: string;
fieldMap: Record<string, IFieldInstance>;
queryBuilder: Knex.QueryBuilder;
dbTableName: string;
}
): Promise<{ queryBuilder: Knex.QueryBuilder; fieldMap: Record<string, IFieldInstance> }> {
const { fieldMap, baseId, queryBuilder } = context;
const { fieldMap, baseId, queryBuilder, dbTableName } = context;
let currentQueryBuilder = queryBuilder;
let currentFieldMap = fieldMap;
if (baseQuery.join) {
Expand Down Expand Up @@ -207,7 +209,7 @@ export class BaseQueryService {
new QueryAggregation().parse(baseQuery.aggregation, {
queryBuilder: currentQueryBuilder,
fieldMap: currentFieldMap,
dbTableName: '',
dbTableName,
dbProvider: this.dbProvider,
});
currentFieldMap = aggregatedFieldMap;
Expand All @@ -232,6 +234,7 @@ export class BaseQueryService {
aggregation: baseQuery.aggregation,
groupBy: baseQuery.groupBy,
knex: this.knex,
dbProvider: this.dbProvider,
});

return { queryBuilder: selectedQueryBuilder, fieldMap: selectedFieldMap };
Expand Down
19 changes: 14 additions & 5 deletions apps/nestjs-backend/src/features/base/base-query/parse/select.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@ import { BaseQueryColumnType } from '@teable/openapi';
import type { IQueryAggregation, IBaseQuerySelect, IBaseQueryGroupBy } from '@teable/openapi';
import type { Knex } from 'knex';
import { cloneDeep, isEmpty } from 'lodash';
import type { IDbProvider } from '../../../../db-provider/db.provider.interface';
import { isUserOrLink } from '../../../../utils/is-user-or-link';
import type { IFieldInstance } from '../../../field/model/factory';
import { getQueryColumnTypeByFieldInstance } from './utils';

Expand All @@ -15,16 +17,18 @@ export class QuerySelect {
fieldMap: Record<string, IFieldInstance>;
aggregation: IQueryAggregation | undefined;
groupBy: IBaseQueryGroupBy | undefined;
dbProvider: IDbProvider;
}
): { queryBuilder: Knex.QueryBuilder; fieldMap: Record<string, IFieldInstance> } {
const { queryBuilder, fieldMap, groupBy, aggregation, knex } = content;
const { queryBuilder, fieldMap, groupBy, aggregation, knex, dbProvider } = content;
let currentFieldMap = cloneDeep(fieldMap);

// column must appear in the GROUP BY clause or be used in an aggregate function
const groupFieldMap = this.selectGroup(queryBuilder, {
knex,
groupBy,
fieldMap: currentFieldMap,
dbProvider,
});
const allowSelectColumnIds = this.allowSelectedColumnIds(currentFieldMap, groupBy, aggregation);
if (aggregation?.length || groupBy?.length) {
Expand Down Expand Up @@ -160,9 +164,10 @@ export class QuerySelect {
groupBy: IBaseQueryGroupBy | undefined;
fieldMap: Record<string, IFieldInstance>;
knex: Knex;
dbProvider: IDbProvider;
}
): Record<string, IFieldInstance> | undefined {
const { groupBy, fieldMap, knex } = content;
const { groupBy, fieldMap, knex, dbProvider } = content;
if (!groupBy) {
return;
}
Expand All @@ -176,13 +181,17 @@ export class QuerySelect {
{} as Record<string, IFieldInstance>
);
const groupByColumnMap = this.extractGroupByColumnMap(queryBuilder, groupFieldMap);
Object.entries(groupByColumnMap).forEach(([dbFieldName, column]) => {
Object.entries(groupByColumnMap).forEach(([fieldId, column]) => {
if (isUserOrLink(fieldMap[fieldId].type)) {
dbProvider.baseQuery().jsonSelect(queryBuilder, fieldMap[fieldId].dbFieldName, fieldId);
return;
}
queryBuilder.select(
typeof column === 'string'
? {
[dbFieldName]: column,
[fieldId]: column,
}
: knex.raw(`${column.sql} as ??`, [...column.bindings, dbFieldName])
: knex.raw(`${column.sql} as ??`, [...column.bindings, fieldId])
);
});

Expand Down
35 changes: 35 additions & 0 deletions apps/nestjs-backend/test/base-query.e2e-spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -247,6 +247,41 @@ describe('BaseSqlQuery e2e', () => {
);
});

it('groupBy with single user field', async () => {
const table = await createTable(baseId, {
fields: [
{
name: 'user',
type: FieldType.User,
},
],
records: [
{
fields: {},
},
{
fields: {
user: {
id: globalThis.testConfig.userId,
title: globalThis.testConfig.userName,
email: globalThis.testConfig.email,
},
},
},
],
}).then((res) => res.data);
const res = await baseQuery(baseId, {
from: table.id,
groupBy: [{ column: table.fields[0].id, type: BaseQueryColumnType.Field }],
});
console.log('res.data', res.data);
expect(res.data.columns).toHaveLength(1);
expect(res.data.rows).toEqual([
{},
{ [`${table.fields[0].id}`]: globalThis.testConfig.userName },
]);
});

it('limit and offset', async () => {
const res = await baseQuery(baseId, {
from: table.id,
Expand Down

0 comments on commit 1038cb9

Please sign in to comment.