Skip to content

Commit 9766248

Browse files
pregnantboypregnantboy
committed
fix: numeric comparisons for tiles v2
1 parent 5d9efdb commit 9766248

File tree

2 files changed

+170
-17
lines changed

2 files changed

+170
-17
lines changed

‎packages/backend/src/models/tiles/pg/__tests__/table-row-functions.itest.ts‎

Lines changed: 147 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -386,18 +386,23 @@ describe('table-row-functions', () => {
386386
describe('getTableRows', () => {
387387
let dataArray: Record<string, string>[]
388388
let rowIds: string[]
389+
const NUM_ROWS = 9
389390
beforeEach(async () => {
390391
// Add test rows for filtering tests
391-
dataArray = new Array(5).fill(0).map(() =>
392+
dataArray = new Array(NUM_ROWS).fill(0).map(() =>
392393
generateMockTableRowData({
393394
columnIds: dummyColumnIds,
394395
}),
395396
)
396-
dataArray[0][dummyColumnIds[3]] = '0'
397+
dataArray[0][dummyColumnIds[3]] = '5'
397398
dataArray[1][dummyColumnIds[3]] = '10'
398399
dataArray[2][dummyColumnIds[3]] = '20'
399400
dataArray[3][dummyColumnIds[3]] = '30'
400401
dataArray[4][dummyColumnIds[3]] = '40'
402+
dataArray[5][dummyColumnIds[3]] = '140'
403+
dataArray[6][dummyColumnIds[3]] = 'abc'
404+
dataArray[7][dummyColumnIds[3]] = 'def'
405+
dataArray[8][dummyColumnIds[3]] = 'ghi'
401406
rowIds = await createTableRows({
402407
tableId: dummyTable.id,
403408
dataArray,
@@ -406,7 +411,7 @@ describe('table-row-functions', () => {
406411

407412
it('should return all rows when no filters are specified', async () => {
408413
const result = await getTableRows({ tableId: dummyTable.id })
409-
expect(result.rows).toHaveLength(5)
414+
expect(result.rows).toHaveLength(NUM_ROWS)
410415
})
411416

412417
it('should return only specified columns', async () => {
@@ -415,7 +420,7 @@ describe('table-row-functions', () => {
415420
columnIds: [dummyColumnIds[0]],
416421
})
417422

418-
expect(result.rows).toHaveLength(5)
423+
expect(result.rows).toHaveLength(NUM_ROWS)
419424
expect(Object.keys(result.rows[0].data)).toEqual([dummyColumnIds[0]])
420425
})
421426

@@ -458,42 +463,171 @@ describe('table-row-functions', () => {
458463
{
459464
columnId: dummyColumnIds[3],
460465
operator: TableRowFilterOperator.GreaterThan,
461-
value: '25',
466+
value: '20',
462467
},
463468
],
464469
})
465470

466-
expect(result.rows).toHaveLength(2)
471+
expect(result.rows).toHaveLength(3)
467472
expect(result.rows.map((r) => r.rowId).sort()).toEqual([
468473
rowIds[3],
469474
rowIds[4],
475+
rowIds[5],
470476
])
477+
478+
const result2 = await getTableRows({
479+
tableId: dummyTable.id,
480+
filters: [
481+
{
482+
columnId: dummyColumnIds[3],
483+
operator: TableRowFilterOperator.GreaterThan,
484+
value: 'abc',
485+
},
486+
],
487+
})
488+
489+
expect(result2.rows).toHaveLength(2)
490+
expect(result2.rows.map((r) => r.rowId).sort()).toEqual([
491+
rowIds[7],
492+
rowIds[8],
493+
])
494+
})
495+
496+
it('should filter rows with greaterThanOrEquals operator', async () => {
497+
const result = await getTableRows({
498+
tableId: dummyTable.id,
499+
filters: [
500+
{
501+
columnId: dummyColumnIds[3],
502+
operator: TableRowFilterOperator.GreaterThanOrEquals,
503+
value: '20',
504+
},
505+
],
506+
})
507+
508+
expect(result.rows).toHaveLength(4)
509+
expect(result.rows.map((r) => r.rowId).sort()).toEqual([
510+
rowIds[2],
511+
rowIds[3],
512+
rowIds[4],
513+
rowIds[5],
514+
])
515+
516+
const result2 = await getTableRows({
517+
tableId: dummyTable.id,
518+
filters: [
519+
{
520+
columnId: dummyColumnIds[3],
521+
operator: TableRowFilterOperator.GreaterThanOrEquals,
522+
value: 'abc',
523+
},
524+
],
525+
})
526+
527+
expect(result2.rows).toHaveLength(3)
528+
expect(result2.rows.map((r) => r.rowId).sort()).toEqual([
529+
rowIds[6],
530+
rowIds[7],
531+
rowIds[8],
532+
])
533+
})
534+
535+
it('should filter rows with lessThan operator', async () => {
536+
const result = await getTableRows({
537+
tableId: dummyTable.id,
538+
filters: [
539+
{
540+
columnId: dummyColumnIds[3],
541+
operator: TableRowFilterOperator.LessThan,
542+
value: '30',
543+
},
544+
],
545+
})
546+
547+
expect(result.rows).toHaveLength(3)
548+
expect(result.rows.map((r) => r.rowId).sort()).toEqual([
549+
rowIds[0],
550+
rowIds[1],
551+
rowIds[2],
552+
])
553+
554+
const result2 = await getTableRows({
555+
tableId: dummyTable.id,
556+
filters: [
557+
{
558+
columnId: dummyColumnIds[3],
559+
operator: TableRowFilterOperator.LessThan,
560+
value: 'aaa',
561+
},
562+
],
563+
})
564+
565+
expect(result2.rows).toHaveLength(6)
566+
expect(result2.rows.map((r) => r.rowId).sort()).toEqual(
567+
rowIds.slice(0, -3),
568+
)
569+
})
570+
571+
it('should filter rows with lessThanOrEquals operator', async () => {
572+
const result = await getTableRows({
573+
tableId: dummyTable.id,
574+
filters: [
575+
{
576+
columnId: dummyColumnIds[3],
577+
operator: TableRowFilterOperator.LessThanOrEquals,
578+
value: '20',
579+
},
580+
],
581+
})
582+
583+
expect(result.rows).toHaveLength(3)
584+
expect(result.rows.map((r) => r.rowId).sort()).toEqual([
585+
rowIds[0],
586+
rowIds[1],
587+
rowIds[2],
588+
])
589+
590+
const result2 = await getTableRows({
591+
tableId: dummyTable.id,
592+
filters: [
593+
{
594+
columnId: dummyColumnIds[3],
595+
operator: TableRowFilterOperator.LessThanOrEquals,
596+
value: 'abc',
597+
},
598+
],
599+
})
600+
601+
expect(result2.rows).toHaveLength(7)
602+
expect(result2.rows.map((r) => r.rowId).sort()).toEqual(
603+
rowIds.slice(0, -2),
604+
)
471605
})
472606

473607
it('should return rows with pagination using scanLimit and cursor', async () => {
474608
// First page
475609
const firstPage = await getTableRows({
476610
tableId: dummyTable.id,
477-
scanLimit: 2,
611+
scanLimit: 4,
478612
})
479613

480-
expect(firstPage.rows).toHaveLength(2)
614+
expect(firstPage.rows).toHaveLength(4)
481615
expect(firstPage.stringifiedCursor).not.toBeNull()
482616

483617
// Second page
484618
const secondPage = await getTableRows({
485619
tableId: dummyTable.id,
486-
scanLimit: 2,
620+
scanLimit: 4,
487621
stringifiedCursor: firstPage.stringifiedCursor,
488622
})
489623

490-
expect(secondPage.rows).toHaveLength(2)
624+
expect(secondPage.rows).toHaveLength(4)
491625
expect(secondPage.stringifiedCursor).not.toBeNull()
492626

493627
// Third page
494628
const thirdPage = await getTableRows({
495629
tableId: dummyTable.id,
496-
scanLimit: 2,
630+
scanLimit: 4,
497631
stringifiedCursor: secondPage.stringifiedCursor,
498632
})
499633

@@ -509,8 +643,8 @@ describe('table-row-functions', () => {
509643

510644
// Since rows are ordered by rowId, which follows creation order,
511645
// we expect the names to be in reverse creation order
512-
expect(result.rows[0].rowId).toBe(rowIds[4])
513-
expect(result.rows[result.rows.length - 1].rowId).toBe(rowIds[0])
646+
expect(result.rows[0].rowId).toBe(rowIds[NUM_ROWS - 1])
647+
expect(result.rows[NUM_ROWS - 1].rowId).toBe(rowIds[0])
514648
})
515649
})
516650

‎packages/backend/src/models/tiles/pg/table-row-functions.ts‎

Lines changed: 23 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -190,7 +190,26 @@ function addFiltersToQuery(
190190
query: Knex.QueryBuilder,
191191
filters: TableRowFilter[],
192192
) {
193+
// This regex is used to validate that a string is a valid number
194+
// It matches strings that start with an optional minus sign, followed by an optional zero,
195+
// or any number from 1 to 9 followed by any number of digits, optionally followed by a decimal point and any number of digits.
196+
// Examples: "123", "-456", "0.789", "-0.123", "123.456", "-456.789"
197+
const VALID_NUMBER_REGEX = '^-?(0|[1-9]\\d*)(\\.\\d+)?$'
198+
193199
for (const filter of filters) {
200+
const isValueNumber = !isNaN(+filter.value)
201+
202+
// use raw expression here to avoid type errors
203+
let castedColumn: Knex.Raw = tilesClient.raw('??', [filter.columnId])
204+
let castedValue: number | string = filter.value
205+
206+
// if the value to compare against is a number, we case the stored values to a number if
207+
// they are valid numeric strings, or else we compare them string to string
208+
if (isValueNumber) {
209+
query.where(filter.columnId, '~', VALID_NUMBER_REGEX)
210+
castedColumn = tilesClient.raw('??::numeric', [filter.columnId])
211+
castedValue = +filter.value
212+
}
194213
switch (filter.operator) {
195214
case TableRowFilterOperator.Equals:
196215
query.where(filter.columnId, '=', filter.value)
@@ -199,16 +218,16 @@ function addFiltersToQuery(
199218
query.where(filter.columnId, 'ilike', `%${filter.value}%`)
200219
break
201220
case TableRowFilterOperator.GreaterThan:
202-
query.where(filter.columnId, '>', filter.value)
221+
query.where(castedColumn, '>', castedValue)
203222
break
204223
case TableRowFilterOperator.GreaterThanOrEquals:
205-
query.where(filter.columnId, '>=', filter.value)
224+
query.where(castedColumn, '>=', castedValue)
206225
break
207226
case TableRowFilterOperator.LessThan:
208-
query.where(filter.columnId, '<', filter.value)
227+
query.where(castedColumn, '<', castedValue)
209228
break
210229
case TableRowFilterOperator.LessThanOrEquals:
211-
query.where(filter.columnId, '<=', filter.value)
230+
query.where(castedColumn, '<=', castedValue)
212231
break
213232
case TableRowFilterOperator.IsEmpty:
214233
query.where((builder) => {

0 commit comments

Comments
 (0)