Skip to content

Commit dc21063

Browse files
nichenqinnichenqin
authored
Merge pull request #2079 from teableio/fix/formula-issues
fix/formula issues
2 parents 806f707 + 6b61eba commit dc21063

30 files changed

+1288
-127
lines changed

‎apps/nestjs-backend/src/db-provider/aggregation-query/postgres/__tests__/multiple-value-aggregation.adapter.spec.ts‎

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,40 @@
1+
import type { FieldCore } from '@teable/core';
2+
import { FieldType } from '@teable/core';
3+
import knex from 'knex';
4+
import { describe, expect, it } from 'vitest';
5+
import type { IRecordQueryAggregateContext } from '../../../../features/record/query-builder/record-query-builder.interface';
6+
import { MultipleValueAggregationAdapter } from '../multiple-value/multiple-value-aggregation.adapter';
7+
8+
const knexClient = knex({ client: 'pg' });
9+
10+
const createAdapter = () => {
11+
const field = {
12+
id: 'fldNumericArray',
13+
dbFieldName: '"values"',
14+
isMultipleCellValue: true,
15+
type: FieldType.Number,
16+
} as unknown as FieldCore;
17+
18+
const context: IRecordQueryAggregateContext = {
19+
selectionMap: new Map([[field.id, '"alias"."values"']]),
20+
tableDbName: 'public.test_table',
21+
tableAlias: 'alias',
22+
};
23+
24+
return new MultipleValueAggregationAdapter(knexClient, field, context);
25+
};
26+
27+
describe('MultipleValueAggregationAdapter numeric coercion', () => {
28+
it.each([
29+
['sum', (adapter: MultipleValueAggregationAdapter) => adapter.sum()],
30+
['average', (adapter: MultipleValueAggregationAdapter) => adapter.average()],
31+
['max', (adapter: MultipleValueAggregationAdapter) => adapter.max()],
32+
['min', (adapter: MultipleValueAggregationAdapter) => adapter.min()],
33+
])('renders %s aggregation without integer casts', (_, getSql) => {
34+
const adapter = createAdapter();
35+
const sql = getSql(adapter);
36+
expect(sql).toContain('::double precision');
37+
expect(sql).toContain('REGEXP_REPLACE');
38+
expect(sql.toUpperCase()).not.toContain('::INTEGER');
39+
});
40+
});

‎apps/nestjs-backend/src/db-provider/aggregation-query/postgres/multiple-value/multiple-value-aggregation.adapter.ts‎

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,12 @@
11
import { AggregationFunctionPostgres } from '../aggregation-function.postgres';
22

33
export class MultipleValueAggregationAdapter extends AggregationFunctionPostgres {
4+
private toNumericSafe(columnExpression: string): string {
5+
const textExpr = `(${columnExpression})::text COLLATE "C"`;
6+
const sanitized = `REGEXP_REPLACE(${textExpr}, '[^0-9.+-]', '', 'g')`;
7+
return `NULLIF(${sanitized}, '' COLLATE "C")::double precision`;
8+
}
9+
410
unique(): string {
511
return this.knex
612
.raw(
@@ -13,7 +19,7 @@ export class MultipleValueAggregationAdapter extends AggregationFunctionPostgres
1319
max(): string {
1420
return this.knex
1521
.raw(
16-
`SELECT MAX("value"::INTEGER) AS "value" FROM ?? as "${this.tableAlias}", jsonb_array_elements_text(${this.tableColumnRef}::jsonb)`,
22+
`SELECT MAX(${this.toNumericSafe('"value"')}) AS "value" FROM ?? as "${this.tableAlias}", jsonb_array_elements_text(${this.tableColumnRef}::jsonb)`,
1723
[this.dbTableName]
1824
)
1925
.toQuery();
@@ -22,7 +28,7 @@ export class MultipleValueAggregationAdapter extends AggregationFunctionPostgres
2228
min(): string {
2329
return this.knex
2430
.raw(
25-
`SELECT MIN("value"::INTEGER) AS "value" FROM ?? as "${this.tableAlias}", jsonb_array_elements_text(${this.tableColumnRef}::jsonb)`,
31+
`SELECT MIN(${this.toNumericSafe('"value"')}) AS "value" FROM ?? as "${this.tableAlias}", jsonb_array_elements_text(${this.tableColumnRef}::jsonb)`,
2632
[this.dbTableName]
2733
)
2834
.toQuery();
@@ -31,7 +37,7 @@ export class MultipleValueAggregationAdapter extends AggregationFunctionPostgres
3137
sum(): string {
3238
return this.knex
3339
.raw(
34-
`SELECT SUM("value"::INTEGER) AS "value" FROM ?? as "${this.tableAlias}", jsonb_array_elements_text(${this.tableColumnRef}::jsonb)`,
40+
`SELECT SUM(${this.toNumericSafe('"value"')}) AS "value" FROM ?? as "${this.tableAlias}", jsonb_array_elements_text(${this.tableColumnRef}::jsonb)`,
3541
[this.dbTableName]
3642
)
3743
.toQuery();
@@ -40,7 +46,7 @@ export class MultipleValueAggregationAdapter extends AggregationFunctionPostgres
4046
average(): string {
4147
return this.knex
4248
.raw(
43-
`SELECT AVG("value"::INTEGER) AS "value" FROM ?? as "${this.tableAlias}", jsonb_array_elements_text(${this.tableColumnRef}::jsonb)`,
49+
`SELECT AVG(${this.toNumericSafe('"value"')}) AS "value" FROM ?? as "${this.tableAlias}", jsonb_array_elements_text(${this.tableColumnRef}::jsonb)`,
4450
[this.dbTableName]
4551
)
4652
.toQuery();

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

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
/* eslint-disable sonarjs/no-duplicate-string */
2+
import { DbFieldType } from '@teable/core';
23
import type { TableDomain } from '@teable/core';
34
import { beforeEach, describe, expect, it } from 'vitest';
45

@@ -185,4 +186,24 @@ describe('GeneratedColumnQueryPostgres unit-aware helpers', () => {
185186
`DATE_TRUNC('${expectedUnit}', date_a::timestamp) = DATE_TRUNC('${expectedUnit}', date_b::timestamp)`
186187
);
187188
});
189+
190+
it('coerces JSON operands before comparing to text columns', () => {
191+
const tableStub = {
192+
fieldList: [
193+
{ dbFieldName: 'text_col', dbFieldType: DbFieldType.Text },
194+
{ dbFieldName: 'json_col', dbFieldType: DbFieldType.Json },
195+
],
196+
} as unknown as TableDomain;
197+
198+
query.setContext({
199+
table: tableStub,
200+
isGeneratedColumn: true,
201+
});
202+
203+
const sql = query.equal('"text_col"', '"json_col"');
204+
205+
expect(sql).toContain('pg_typeof(("json_col")) = ANY');
206+
expect(sql).toContain('jsonb_typeof((("json_col"))::jsonb)');
207+
expect(sql).toContain('("text_col")::text COLLATE "C"');
208+
});
188209
});

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

Lines changed: 60 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -34,11 +34,16 @@ export class GeneratedColumnQueryPostgres extends GeneratedColumnQueryAbstract {
3434
return depth === 0;
3535
}
3636

37-
private isNumericLiteral(expr: string): boolean {
37+
private stripOuterParentheses(expr: string): string {
3838
let trimmed = expr.trim();
3939
while (trimmed.length > 0 && this.hasWrappingParentheses(trimmed)) {
4040
trimmed = trimmed.slice(1, -1).trim();
4141
}
42+
return trimmed;
43+
}
44+
45+
private isNumericLiteral(expr: string): boolean {
46+
const trimmed = this.stripOuterParentheses(expr);
4247
// eslint-disable-next-line regexp/no-unused-capturing-group
4348
return /^[-+]?\d+(\.\d+)?$/.test(trimmed);
4449
}
@@ -53,7 +58,8 @@ export class GeneratedColumnQueryPostgres extends GeneratedColumnQueryAbstract {
5358
}
5459

5560
private normalizeBlankComparable(value: string): string {
56-
return `COALESCE(NULLIF((${value})::text, ''), '')`;
61+
const comparable = this.coerceToTextComparable(value);
62+
return `COALESCE(NULLIF(${comparable}, ''), '')`;
5763
}
5864

5965
private ensureTextCollation(expr: string): string {
@@ -63,7 +69,26 @@ export class GeneratedColumnQueryPostgres extends GeneratedColumnQueryAbstract {
6369
private buildBlankAwareComparison(operator: '=' | '<>', left: string, right: string): string {
6470
const shouldNormalize = this.isEmptyStringLiteral(left) || this.isEmptyStringLiteral(right);
6571
if (!shouldNormalize) {
66-
return `(${left} ${operator} ${right})`;
72+
const leftIsText = this.isTextLikeExpression(left);
73+
const rightIsText = this.isTextLikeExpression(right);
74+
75+
let normalizedLeft = left;
76+
let normalizedRight = right;
77+
78+
if (leftIsText) {
79+
normalizedLeft = this.ensureTextCollation(left);
80+
}
81+
if (rightIsText) {
82+
normalizedRight = this.ensureTextCollation(right);
83+
}
84+
85+
if (leftIsText && !rightIsText) {
86+
normalizedRight = this.coerceToTextComparable(right);
87+
} else if (!leftIsText && rightIsText) {
88+
normalizedLeft = this.coerceToTextComparable(left);
89+
}
90+
91+
return `(${normalizedLeft} ${operator} ${normalizedRight})`;
6792
}
6893

6994
const normalizedLeft = this.isEmptyStringLiteral(left)
@@ -77,13 +102,13 @@ export class GeneratedColumnQueryPostgres extends GeneratedColumnQueryAbstract {
77102
}
78103

79104
private isTextLikeExpression(value: string): boolean {
80-
const trimmed = value.trim();
105+
const trimmed = this.stripOuterParentheses(value);
81106
if (/^'.*'$/.test(trimmed)) {
82107
return true;
83108
}
84109

85-
const columnMatch = trimmed.match(/^"([^"]+)"$/);
86-
if (!columnMatch) {
110+
const columnMatch = trimmed.match(/^"([^"]+)"$/) ?? trimmed.match(/^"[^"]+"\."([^"]+)"$/);
111+
if (!columnMatch || columnMatch.length < 2) {
87112
return false;
88113
}
89114

@@ -99,6 +124,35 @@ export class GeneratedColumnQueryPostgres extends GeneratedColumnQueryAbstract {
99124
return field.dbFieldType === DbFieldType.Text;
100125
}
101126

127+
private coerceToTextComparable(value: string): string {
128+
const trimmed = this.stripOuterParentheses(value);
129+
if (!trimmed) {
130+
return this.ensureTextCollation(value);
131+
}
132+
if (/^'.*'$/.test(trimmed)) {
133+
return this.ensureTextCollation(trimmed);
134+
}
135+
if (trimmed.toUpperCase() === 'NULL') {
136+
return 'NULL';
137+
}
138+
139+
const wrapped = `(${value})`;
140+
const coerced = `(CASE
141+
WHEN ${wrapped} IS NULL THEN NULL
142+
WHEN pg_typeof(${wrapped}) = ANY(ARRAY['jsonb'::regtype, 'json'::regtype]) THEN (
143+
CASE jsonb_typeof((${wrapped})::jsonb)
144+
WHEN 'string' THEN (${wrapped})::jsonb #>> '{}'
145+
WHEN 'number' THEN (${wrapped})::jsonb #>> '{}'
146+
WHEN 'boolean' THEN (${wrapped})::jsonb #>> '{}'
147+
WHEN 'null' THEN NULL
148+
ELSE (${wrapped})::jsonb::text
149+
END
150+
)
151+
ELSE ${wrapped}::text
152+
END)`;
153+
return this.ensureTextCollation(coerced);
154+
}
155+
102156
private countANonNullExpression(value: string): string {
103157
if (this.isTextLikeExpression(value)) {
104158
const normalizedComparable = this.normalizeBlankComparable(value);

‎apps/nestjs-backend/src/db-provider/select-query/postgres/select-query.postgres.spec.ts‎

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -337,3 +337,28 @@ describe('Select formula boolean normalization', () => {
337337
expect(sql).not.toContain('::boolean');
338338
});
339339
});
340+
341+
describe('Select formula string comparisons', () => {
342+
const query = new SelectQueryPostgres();
343+
const tableStub = {
344+
fieldList: [
345+
{ dbFieldName: 'text_col', dbFieldType: DbFieldType.Text },
346+
{ dbFieldName: 'json_col', dbFieldType: DbFieldType.Json },
347+
],
348+
} as unknown as TableDomain;
349+
350+
const buildContext = (): ISelectFormulaConversionContext => ({
351+
table: tableStub,
352+
selectionMap: new Map<string, IFieldSelectName>(),
353+
});
354+
355+
it('coerces JSON operands to text when compared with text columns', () => {
356+
query.setContext(buildContext());
357+
const sql = query.equal('"main"."text_col"', '"main"."json_col"');
358+
359+
expect(sql).toContain('pg_typeof(("main"."json_col")) = ANY');
360+
expect(sql).toContain('jsonb_typeof((("main"."json_col"))::jsonb)');
361+
expect(sql).toContain('("main"."text_col")::text COLLATE "C"');
362+
expect(sql).not.toContain('= "main"."json_col"');
363+
});
364+
});

‎apps/nestjs-backend/src/db-provider/select-query/postgres/select-query.postgres.ts‎

Lines changed: 60 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -42,11 +42,16 @@ export class SelectQueryPostgres extends SelectQueryAbstract {
4242
return depth === 0;
4343
}
4444

45-
private isNumericLiteral(expr: string): boolean {
45+
private stripOuterParentheses(expr: string): string {
4646
let trimmed = expr.trim();
4747
while (trimmed.length > 0 && this.hasWrappingParentheses(trimmed)) {
4848
trimmed = trimmed.slice(1, -1).trim();
4949
}
50+
return trimmed;
51+
}
52+
53+
private isNumericLiteral(expr: string): boolean {
54+
const trimmed = this.stripOuterParentheses(expr);
5055
// eslint-disable-next-line regexp/no-unused-capturing-group
5156
return /^[-+]?\d+(\.\d+)?$/.test(trimmed);
5257
}
@@ -73,21 +78,22 @@ export class SelectQueryPostgres extends SelectQueryAbstract {
7378
}
7479

7580
private normalizeBlankComparable(value: string): string {
76-
return `COALESCE(NULLIF((${value})::text, ''), '')`;
81+
const comparable = this.coerceToTextComparable(value);
82+
return `COALESCE(NULLIF(${comparable}, ''), '')`;
7783
}
7884

7985
private ensureTextCollation(expr: string): string {
8086
return `(${expr})::text COLLATE "C"`;
8187
}
8288

8389
private isTextLikeExpression(value: string): boolean {
84-
const trimmed = value.trim();
90+
const trimmed = this.stripOuterParentheses(value);
8591
if (/^'.*'$/.test(trimmed)) {
8692
return true;
8793
}
8894

89-
const columnMatch = trimmed.match(/^"([^"]+)"$/);
90-
if (!columnMatch) {
95+
const columnMatch = trimmed.match(/^"([^"]+)"$/) ?? trimmed.match(/^"[^"]+"\."([^"]+)"$/);
96+
if (!columnMatch || columnMatch.length < 2) {
9197
return false;
9298
}
9399

@@ -103,6 +109,35 @@ export class SelectQueryPostgres extends SelectQueryAbstract {
103109
return field.dbFieldType === DbFieldType.Text;
104110
}
105111

112+
private coerceToTextComparable(value: string): string {
113+
const trimmed = this.stripOuterParentheses(value);
114+
if (!trimmed) {
115+
return this.ensureTextCollation(value);
116+
}
117+
if (/^'.*'$/.test(trimmed)) {
118+
return this.ensureTextCollation(trimmed);
119+
}
120+
if (trimmed.toUpperCase() === 'NULL') {
121+
return 'NULL';
122+
}
123+
124+
const wrapped = `(${value})`;
125+
const coerced = `(CASE
126+
WHEN ${wrapped} IS NULL THEN NULL
127+
WHEN pg_typeof(${wrapped}) = ANY(ARRAY['jsonb'::regtype, 'json'::regtype]) THEN (
128+
CASE jsonb_typeof((${wrapped})::jsonb)
129+
WHEN 'string' THEN (${wrapped})::jsonb #>> '{}'
130+
WHEN 'number' THEN (${wrapped})::jsonb #>> '{}'
131+
WHEN 'boolean' THEN (${wrapped})::jsonb #>> '{}'
132+
WHEN 'null' THEN NULL
133+
ELSE (${wrapped})::jsonb::text
134+
END
135+
)
136+
ELSE ${wrapped}::text
137+
END)`;
138+
return this.ensureTextCollation(coerced);
139+
}
140+
106141
private countANonNullExpression(value: string): string {
107142
if (this.isTextLikeExpression(value)) {
108143
const normalizedComparable = this.normalizeBlankComparable(value);
@@ -249,7 +284,26 @@ export class SelectQueryPostgres extends SelectQueryAbstract {
249284
private buildBlankAwareComparison(operator: '=' | '<>', left: string, right: string): string {
250285
const shouldNormalize = this.isEmptyStringLiteral(left) || this.isEmptyStringLiteral(right);
251286
if (!shouldNormalize) {
252-
return `(${left} ${operator} ${right})`;
287+
const leftIsText = this.isTextLikeExpression(left);
288+
const rightIsText = this.isTextLikeExpression(right);
289+
290+
let normalizedLeft = left;
291+
let normalizedRight = right;
292+
293+
if (leftIsText) {
294+
normalizedLeft = this.ensureTextCollation(left);
295+
}
296+
if (rightIsText) {
297+
normalizedRight = this.ensureTextCollation(right);
298+
}
299+
300+
if (leftIsText && !rightIsText) {
301+
normalizedRight = this.coerceToTextComparable(right);
302+
} else if (!leftIsText && rightIsText) {
303+
normalizedLeft = this.coerceToTextComparable(left);
304+
}
305+
306+
return `(${normalizedLeft} ${operator} ${normalizedRight})`;
253307
}
254308

255309
const normalizedLeft = this.isEmptyStringLiteral(left)

‎apps/nestjs-backend/src/features/aggregation/aggregation.service.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -254,7 +254,7 @@ export class AggregationService implements IAggregationService {
254254

255255
// Build aggregate query first, then attach permission CTE on the same builder
256256
const { qb, alias } = await this.recordQueryBuilder.createRecordAggregateBuilder(dbTableName, {
257-
tableIdOrDbTableName: tableId,
257+
tableId,
258258
viewId,
259259
filter,
260260
aggregationFields: statisticFields,
@@ -511,7 +511,7 @@ export class AggregationService implements IAggregationService {
511511
const { qb, alias, selectionMap } = await this.recordQueryBuilder.createRecordAggregateBuilder(
512512
dbTableName,
513513
{
514-
tableIdOrDbTableName: tableId,
514+
tableId,
515515
viewId,
516516
currentUserId: withUserId,
517517
filter,

0 commit comments

Comments
 (0)