Skip to content

Commit 66ab0a9

Browse files
committed
fix(tables): gate the v2 filter pruner on the v2 grammar
`prunePredicateForColumns` guards a saved predicate against operators a select column no longer accepts, but it checked the legacy UI set. The two grammars are deliberately not 1:1 — `isNull`/`isNotNull` are meaningful on a select column and have no `$` equivalent — so a saved v2 predicate using either was stripped client-side before the rows query ran, silently widening the filter rather than narrowing it. It now asks `predicateOperatorsFor`, the same registry answer the SQL leaf uses, so the pruner and the query can no longer disagree about what is allowed. Its legacy sibling stays on the UI set, which is correctly aligned with the `$` grammar's own allowlist.
1 parent fd39ff8 commit 66ab0a9

2 files changed

Lines changed: 36 additions & 7 deletions

File tree

apps/sim/lib/table/__tests__/column-types-contact.test.ts

Lines changed: 26 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@ import {
2121
pickMetadata,
2222
} from '@/lib/table/column-types'
2323
import { coerceValue } from '@/lib/table/import'
24-
import { filterRulesToFilter } from '@/lib/table/query-builder/converters'
24+
import { filterRulesToFilter, prunePredicateForColumns } from '@/lib/table/query-builder/converters'
2525
import type { ColumnDefinition } from '@/lib/table/types'
2626

2727
const column = (type: ColumnDefinition['type'], extra: Partial<ColumnDefinition> = {}) =>
@@ -296,6 +296,31 @@ describe('review-round-2 regressions', () => {
296296
})
297297
})
298298

299+
describe('review-round-4 regressions', () => {
300+
const selectColumn: ColumnDefinition = {
301+
id: 'c',
302+
name: 'c',
303+
type: 'select',
304+
options: [{ id: 'opt_1', name: 'One' }],
305+
}
306+
307+
it('keeps isNull/isNotNull on a select column through the v2 pruner', () => {
308+
// The v2 SQL leaf accepts them (they are meaningful on any column and have
309+
// no `$` equivalent); gating the pruner on the legacy set stripped them
310+
// client-side before the query ran, silently widening the filter.
311+
for (const op of ['isNull', 'isNotNull'] as const) {
312+
const predicate = { all: [{ field: 'c', op }] } as const
313+
expect(prunePredicateForColumns(predicate, [selectColumn]), op).toEqual(predicate)
314+
}
315+
})
316+
317+
it('still prunes an operator the select grammar genuinely strands', () => {
318+
// A `contains` left behind by a multi -> single toggle must still go.
319+
const predicate = { all: [{ field: 'c', op: 'contains' as const, value: 'opt_1' }] }
320+
expect(prunePredicateForColumns(predicate, [selectColumn])).toBeNull()
321+
})
322+
})
323+
299324
describe('date includeTime', () => {
300325
it('truncates to a calendar day only when includeTime is explicitly false', () => {
301326
const dateOnly = column('date', { includeTime: false })

apps/sim/lib/table/query-builder/converters.ts

Lines changed: 10 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@
55
import { generateShortId } from '@sim/utils/id'
66
import { isRecordLike } from '@sim/utils/object'
77
import { columnMatchesRef } from '@/lib/table/column-keys'
8-
import { columnTypeOf, storesMultipleValues } from '@/lib/table/column-types'
8+
import { columnTypeOf, predicateOperatorsFor, storesMultipleValues } from '@/lib/table/column-types'
99
import { TableQueryValidationError } from '@/lib/table/errors'
1010
import {
1111
MULTI_SELECT_FILTER_OPERATORS,
@@ -125,6 +125,12 @@ export function pruneFilterForColumns(
125125
* Predicate-grammar sibling of {@link pruneFilterForColumns}: drops conditions a
126126
* `select` column no longer accepts (operator stranded by a type/`multiple`
127127
* change), so a stale applied filter can't fail every subsequent rows query.
128+
*
129+
* Gated on `predicateOperatorsFor` — the **v2** allowlist — not on the UI sets
130+
* its legacy sibling uses. The two grammars are deliberately not 1:1:
131+
* `isNull`/`isNotNull` are meaningful on a select column and have no `$`
132+
* equivalent, so gating this on the legacy set stripped a saved predicate that
133+
* the rows query would have accepted, silently widening the filter.
128134
*/
129135
export function prunePredicateForColumns(
130136
predicate: TablePredicate | null,
@@ -138,11 +144,9 @@ export function prunePredicateForColumns(
138144
const rules = predicateToFilterRules(predicate)
139145
const kept = rules.filter((rule) => {
140146
const column = columns.find((c) => columnMatchesRef(c, rule.column))
141-
if (!column || !columnTypeOf(column).storesOpaqueIds) return true
142-
const allowed = storesMultipleValues(column)
143-
? MULTI_SELECT_FILTER_OPERATORS
144-
: SINGLE_SELECT_FILTER_OPERATORS
145-
return allowed.has(rule.operator)
147+
if (!column) return true
148+
const allowed = predicateOperatorsFor(column)
149+
return allowed === null || allowed.has(rule.operator)
146150
})
147151

148152
if (kept.length === rules.length) return predicate

0 commit comments

Comments
 (0)