Add IN filter support for multi-value filtering across all database d…#1700
Merged
Add IN filter support for multi-value filtering across all database d…#1700
Conversation
…rivers
Allow filtering rows where a column value matches any value in a provided set
(e.g., status IN ('active', 'pending')). Adds the `in` criteria to FilterCriteriaEnum
and implements it in all 11 DAOs: Postgres, MySQL, MSSQL, Oracle (Knex whereIn),
IBM DB2, Cassandra (parameterized IN), MongoDB ($in), DynamoDB (IN expression),
Elasticsearch (terms query), ClickHouse (IN clause), and Redis (in-memory check).
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
📝 WalkthroughWalkthroughAdds IN and BETWEEN filter operators across parser, DTOs, controller, and multiple data-access implementations; broadens filtering value types (backend: unknown, agent: string|string[]); normalizes CSV/array inputs and maps new criteria to DB-specific IN/BETWEEN clauses. Tests and status codes updated accordingly. Changes
Sequence DiagramsequenceDiagram
participant Client
participant Parser as Filter Parser
participant Controller
participant DAO as Data Access Object
participant DB as Database
Client->>Controller: request with filters (body or f_<field>__in / __between)
Controller->>Parser: forward request filter data
Parser->>Parser: detect __in / __between, normalize value (array or CSV → array)
Parser->>Controller: emit FilterCriteria + normalized value
Controller->>DAO: call getRowsFromTable with filters
rect rgba(100, 150, 255, 0.5)
DAO->>DAO: map criteria to DB-specific clause (IN / BETWEEN)
end
DAO->>DB: execute query with clause + bound params
DB-->>DAO: return rows
DAO-->>Controller: return filtered rows
Controller-->>Client: respond 200 with rows
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Contributor
There was a problem hiding this comment.
Pull request overview
Adds multi-value filtering support via a new in filter criterion across the shared filter enums, backend filter parsing/DTOs, and multiple DAO implementations (SQL/NoSQL/in-memory).
Changes:
- Added
FilterCriteriaEnum.into shared-code, backend, and agent enums. - Implemented
IN/$in/terms-style filtering in DAOs (KnexwhereIn, MongoDB$in, DynamoDBIN, ClickHouseIN, Cassandra/DB2 parameterizedIN, Redis in-memory match, Elasticsearchterms). - Updated backend filter parsing and DTO typing to accept multi-value inputs.
Reviewed changes
Copilot reviewed 18 out of 18 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
| shared-code/src/shared/enums/filter-criteria.enum.ts | Adds in criterion to shared enum. |
| shared-code/src/data-access-layer/data-access-objects/data-access-object-redis.ts | Adds in-memory in evaluation for Redis DAO. |
| shared-code/src/data-access-layer/data-access-objects/data-access-object-postgres.ts | Adds Knex whereIn handling for Postgres DAO. |
| shared-code/src/data-access-layer/data-access-objects/data-access-object-oracle.ts | Adds Knex whereIn handling for Oracle DAO. |
| shared-code/src/data-access-layer/data-access-objects/data-access-object-mysql.ts | Adds Knex whereIn handling for MySQL DAO. |
| shared-code/src/data-access-layer/data-access-objects/data-access-object-mssql.ts | Adds Knex whereIn handling for MSSQL DAO. |
| shared-code/src/data-access-layer/data-access-objects/data-access-object-mongodb.ts | Adds $in query support for MongoDB DAO. |
| shared-code/src/data-access-layer/data-access-objects/data-access-object-ibmdb2.ts | Adds parameterized SQL IN support for DB2 DAO. |
| shared-code/src/data-access-layer/data-access-objects/data-access-object-elasticsearch.ts | Adds terms query support for Elasticsearch DAO. |
| shared-code/src/data-access-layer/data-access-objects/data-access-object-dynamodb.ts | Adds IN expression support for DynamoDB DAO. |
| shared-code/src/data-access-layer/data-access-objects/data-access-object-clickhouse.ts | Adds IN clause support for ClickHouse DAO. |
| shared-code/src/data-access-layer/data-access-objects/data-access-object-cassandra.ts | Adds parameterized IN support for Cassandra DAO. |
| rocketadmin-agent/src/interfaces/interfaces.ts | Allows filter value to be `string |
| rocketadmin-agent/src/enums/filter-criteria.enum.ts | Adds in criterion to agent enum. |
| backend/src/enums/filter-criteria.enum.ts | Adds in criterion to backend enum. |
| backend/src/entities/table/utils/find-filtering-fields.util.ts | Parses __in filters from query params and broadens body parsing type. |
| backend/src/entities/table/table-datastructures.ts | Broadens FilteringFieldsDs.value to unknown. |
| backend/src/entities/table/application/data-structures/found-table-rows.ds.ts | Broadens FilteringFieldsDs.value to unknown. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+320
to
+324
| const inValues = Array.isArray(filterObject.value) | ||
| ? filterObject.value | ||
| : String(filterObject.value) | ||
| .split(',') | ||
| .map((v) => v.trim()); |
There was a problem hiding this comment.
FilterCriteriaEnum.in can produce an invalid SQL snippet when inValues is empty (e.g., request sends []), because this will generate IN (). Consider validating that the set is non-empty (and/or trimming+filtering out empty strings) before building placeholders; otherwise return a clear error or short-circuit to a no-match condition.
Suggested change
| const inValues = Array.isArray(filterObject.value) | |
| ? filterObject.value | |
| : String(filterObject.value) | |
| .split(',') | |
| .map((v) => v.trim()); | |
| const rawInValues = Array.isArray(filterObject.value) | |
| ? filterObject.value | |
| : String(filterObject.value).split(','); | |
| const inValues = rawInValues | |
| .map((v) => (typeof v === 'string' ? v.trim() : v)) | |
| .filter((v) => !(typeof v === 'string' && v.length === 0)); | |
| if (inValues.length === 0) { | |
| throw new Error(`Invalid filter values for "${filterObject.field}": IN filter requires at least one non-empty value`); | |
| } |
Comment on lines
+283
to
+292
| case FilterCriteriaEnum.in: { | ||
| const inValues = Array.isArray(filter.value) | ||
| ? filter.value | ||
| : String(filter.value) | ||
| .split(',') | ||
| .map((v) => v.trim()); | ||
| const placeholders = inValues.map(() => '?').join(', '); | ||
| whereConditions.push(`${filter.field.toLowerCase()} IN (${placeholders})`); | ||
| params.push(...inValues); | ||
| break; |
There was a problem hiding this comment.
If inValues is empty, placeholders becomes an empty string and the generated CQL will be ... IN (), which is invalid. Add a guard for an empty set (e.g., throw a validation error or short-circuit the query to return zero rows) and consider dropping empty strings after trimming.
Comment on lines
+265
to
+271
| const inValues = Array.isArray(value) | ||
| ? value | ||
| : String(value) | ||
| .split(',') | ||
| .map((v) => v.trim()); | ||
| const sanitizedValues = inValues.map((v) => this.sanitizeValue(v)).join(', '); | ||
| whereClauses.push(`${escapedField} IN (${sanitizedValues})`); |
There was a problem hiding this comment.
FilterCriteriaEnum.in builds ... IN (${sanitizedValues}). When inValues is empty this becomes IN (), which is invalid ClickHouse SQL. Add a guard for empty sets (and optionally filter out empty strings after .trim()) to avoid runtime query failures.
Suggested change
| const inValues = Array.isArray(value) | |
| ? value | |
| : String(value) | |
| .split(',') | |
| .map((v) => v.trim()); | |
| const sanitizedValues = inValues.map((v) => this.sanitizeValue(v)).join(', '); | |
| whereClauses.push(`${escapedField} IN (${sanitizedValues})`); | |
| const inValues = (Array.isArray(value) ? value : String(value).split(',')) | |
| .map((v) => String(v).trim()) | |
| .filter((v) => v.length > 0); | |
| if (inValues.length > 0) { | |
| const sanitizedValues = inValues.map((v) => this.sanitizeValue(v)).join(', '); | |
| whereClauses.push(`${escapedField} IN (${sanitizedValues})`); | |
| } |
Comment on lines
+348
to
+359
| const inValues = Array.isArray(value) | ||
| ? value | ||
| : String(value) | ||
| .split(',') | ||
| .map((v) => v.trim()); | ||
| const placeholders = inValues.map((_, i) => `${uniquePlaceholder}_${i}`); | ||
| filterExpression += ` AND #${field} IN (${placeholders.join(', ')})`; | ||
| inValues.forEach((val, i) => { | ||
| expressionAttributeValues[`${uniquePlaceholder}_${i}`] = isNumberField | ||
| ? { N: String(val) } | ||
| : { S: String(val) }; | ||
| }); |
There was a problem hiding this comment.
When inValues is empty, this generates #field IN (), which is not a valid DynamoDB filter expression. Add a guard for empty sets (and consider filtering out empty strings after trimming) so the request fails with a clear error or short-circuits to no results.
| : String(value) | ||
| .split(',') | ||
| .map((v) => v.trim()); | ||
| acc[field] = { $in: inValues }; |
There was a problem hiding this comment.
This assigns acc[field] = { $in: inValues }, which overwrites any previously accumulated range operators for the same field ($gt/$lt/$gte/$lte). To preserve existing behavior for combined range filters, consider setting $in on the existing object (when it is an object) instead of replacing it.
Suggested change
| acc[field] = { $in: inValues }; | |
| if ( | |
| acc[field] && | |
| typeof acc[field] === 'object' && | |
| !(acc[field] instanceof RegExp) && | |
| !Array.isArray(acc[field]) | |
| ) { | |
| acc[field].$in = inValues; | |
| } else { | |
| acc[field] = { $in: inValues }; | |
| } |
Comment on lines
+93
to
+101
| filteringItems.push({ | ||
| field: fieldname, | ||
| criteria: FilterCriteriaEnum.in, | ||
| value: Array.isArray(rawValue) | ||
| ? rawValue | ||
| : String(rawValue) | ||
| .split(',') | ||
| .map((v) => v.trim()), | ||
| }); |
There was a problem hiding this comment.
__in values are split and trimmed, but empty entries are not removed and empty arrays are allowed. Downstream DAOs may generate invalid queries for an empty set (e.g., IN ()). Consider filtering out empty strings and rejecting/handling the empty-set case here so behavior is consistent across drivers.
Suggested change
| filteringItems.push({ | |
| field: fieldname, | |
| criteria: FilterCriteriaEnum.in, | |
| value: Array.isArray(rawValue) | |
| ? rawValue | |
| : String(rawValue) | |
| .split(',') | |
| .map((v) => v.trim()), | |
| }); | |
| const inValues = (Array.isArray(rawValue) | |
| ? rawValue.map((value) => (typeof value === 'string' ? value.trim() : value)) | |
| : String(rawValue) | |
| .split(',') | |
| .map((value) => value.trim()) | |
| ).filter((value) => value !== ''); | |
| if (inValues.length > 0) { | |
| filteringItems.push({ | |
| field: fieldname, | |
| criteria: FilterCriteriaEnum.in, | |
| value: inValues, | |
| }); | |
| } |
Comment on lines
12
to
+16
| @ApiProperty({ enum: FilterCriteriaEnum }) | ||
| criteria: FilterCriteriaEnum; | ||
|
|
||
| @ApiProperty() | ||
| value: string; | ||
| value: unknown; |
There was a problem hiding this comment.
FilteringFieldsDs.value was widened to unknown, which makes the public API/Swagger schema much less informative. If the intent is to support multi-value filters, consider narrowing this to a union (e.g., string | string[] or string | number | boolean | null | (string|number)[]) and updating the @ApiProperty metadata accordingly.
Comment on lines
122
to
+130
| export class FilteringFieldsDs { | ||
| @ApiProperty({ enum: FilterCriteriaEnum }) | ||
| criteria: FilterCriteriaEnum; | ||
|
|
||
| @ApiProperty() | ||
| field: string; | ||
|
|
||
| @ApiProperty() | ||
| value: string; | ||
| value: unknown; |
There was a problem hiding this comment.
FilteringFieldsDs.value is now unknown, which weakens the generated Swagger schema for this response DTO. Consider using a narrower union type and @ApiProperty({ oneOf: ... }) / isArray metadata so clients can understand expected scalar vs array shapes for in filters.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (5)
rocketadmin-agent/src/interfaces/interfaces.ts (1)
71-72: ReplaceanyinIMessageDataInfopayload fields.
rowandprimaryKeyshould be typed as structured unknown objects (or precise generics) to preserve type safety in message handling.As per coding guidelines "Avoid any types - use specific types or generics instead".Suggested type-tightening
export interface IMessageDataInfo { operationType: OperationTypeEnum; tableName: string; - row: any; - primaryKey: any; + row: Record<string, unknown>; + primaryKey: Record<string, unknown>; tableSettings: TableSettingsDS;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@rocketadmin-agent/src/interfaces/interfaces.ts` around lines 71 - 72, IMessageDataInfo currently uses `any` for `row` and `primaryKey`; change it to a typed generic or structured unknown to restore type safety (e.g., make the interface generic like IMessageDataInfo<TRow = Record<string, unknown>, TKey = Record<string, unknown>> and set `row: TRow` and `primaryKey: TKey`), and update callers to provide concrete type parameters or accept the default Record<string, unknown>; ensure you update references to IMessageDataInfo to pass types where needed so the compiler catches incorrect message payload shapes.backend/src/entities/table/table-datastructures.ts (1)
15-16: DocumentFilteringFieldsDs.valueexplicitly in Swagger.
unknownwith bare@ApiProperty()makes the API contract opaque. Since__inis now supported, expose a concreteoneOfschema (single value vs array) for client generators and docs.Swagger schema refinement example
- `@ApiProperty`() + `@ApiProperty`({ + oneOf: [ + { type: 'string' }, + { type: 'number' }, + { type: 'boolean' }, + { + type: 'array', + items: { + oneOf: [{ type: 'string' }, { type: 'number' }, { type: 'boolean' }], + }, + }, + ], + nullable: true, + }) value: unknown;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/src/entities/table/table-datastructures.ts` around lines 15 - 16, The Swagger contract for FilteringFieldsDs.value is opaque because it's typed as unknown with a bare `@ApiProperty`(); replace the bare decorator with an explicit schema that documents the allowed single-value types and the __in array form: update the `@ApiProperty` on the FilteringFieldsDs.value field to use a oneOf that lists primitive single-value schemas (e.g., string, number, boolean) and an array variant whose items use the same oneOf for the supported primitives, so client generators see "single value OR array of values" documented.shared-code/src/data-access-layer/data-access-objects/data-access-object-postgres.ts (1)
673-682: Duplicated IN filter logic - same pattern as MSSQL.The filtering logic in
getTableRowsStream(lines 649-684) duplicatesgetRowsFromTable(lines 205-239). This is the same duplication pattern seen in the MSSQL DAO. Consider extracting shared filtering logic to a helper method.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@shared-code/src/data-access-layer/data-access-objects/data-access-object-postgres.ts` around lines 673 - 682, The IN-filter branching in getTableRowsStream duplicates the same logic found in getRowsFromTable; extract the shared filter application into a new helper (e.g., applyFilters or buildWhereClauses) that accepts the query builder and the filter definition(s) and handles FilterCriteriaEnum.in (array/string comma-split) and the other criteria using operators[criteria] and values[criteria] || value; then replace the duplicated blocks in getTableRowsStream and getRowsFromTable with calls to that helper so both functions delegate filter construction to the single utility.backend/src/entities/table/utils/find-filtering-fields.util.ts (1)
114-125:parseFilteringFieldsFromBodyDatadoesn't normalize IN values to arrays.Unlike
findFilteringFieldsUtil(lines 91-102) which explicitly normalizes__invalues to arrays,parseFilteringFieldsFromBodyDatapassesfilterData[key]directly at line 124. If a client sends{ "status": { "in": "active,pending" } }as a string instead of an array, it won't be split.While DAOs have defensive normalization, this creates inconsistency between the two parsing paths.
Suggested fix to normalize IN values in body data
for (const key in filterData) { if (!validateStringWithEnum(key, FilterCriteriaEnum)) { throw new Error(`Invalid filter criteria: "${key}".`); } const isValueNull = filterData[key] === null && key === FilterCriteriaEnum.eq; + let filterValue = filterData[key]; + if (key === FilterCriteriaEnum.in && !Array.isArray(filterValue)) { + filterValue = String(filterValue).split(',').map((v) => v.trim()); + } filteringItems.push({ field: rowName, criteria: isValueNull ? FilterCriteriaEnum.empty : (key as FilterCriteriaEnum), - value: filterData[key], + value: filterValue, }); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/src/entities/table/utils/find-filtering-fields.util.ts` around lines 114 - 125, parseFilteringFieldsFromBodyData currently pushes filterData[key] directly (in the loop inside parseFilteringFieldsFromBodyData) which fails to normalize "__in" values when clients pass a comma-separated string; mirror the behavior in findFilteringFieldsUtil by detecting when key === FilterCriteriaEnum.in and normalizing the value before pushing: if the value is a string, split on commas and trim into an array; if it's a non-array single value, wrap it in an array; otherwise leave arrays as-is; then use that normalized value in the filteringItems.push call (preserve the existing handling for FilterCriteriaEnum.eq/empty).shared-code/src/data-access-layer/data-access-objects/data-access-object-mssql.ts (1)
605-614: Duplicated IN filter logic betweengetRowsFromTableandgetTableRowsStream.This filtering logic block (lines 582-616) is nearly identical to the one in
getRowsFromTable(lines 197-231). Consider extracting a shared helper method to reduce duplication and ensure consistency when filter logic changes.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@shared-code/src/data-access-layer/data-access-objects/data-access-object-mssql.ts` around lines 605 - 614, The IN-filter and general filter building code duplicated in getRowsFromTable and getTableRowsStream should be extracted into a shared helper (e.g., buildFilterClause or applyFilterToBuilder) that accepts the query builder, field, criteria, value, operators, and values; move the logic that handles FilterCriteriaEnum.in (normalizing inValues from arrays or comma strings and calling builder.whereIn) and the fallback builder.where into that helper, then replace the duplicated blocks in both getRowsFromTable and getTableRowsStream with calls to the new helper so both methods use the same centralized filter logic.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@rocketadmin-agent/src/interfaces/interfaces.ts`:
- Around line 1-4: Update the inconsistent import paths so all four imports use
the source path pattern (change the imports of AutocompleteFieldsDS,
FilteringFieldsDS, TableSettingsDS to import from
'@rocketadmin/shared-code/src/...') to match TableStructureDS; then in the
IMessageDataInfo interface replace the loose any types on the properties row and
primaryKey with stricter types (use row: Record<string, unknown> and primaryKey:
string | number) to provide type safety and satisfy TypeScript guidelines.
In
`@shared-code/src/data-access-layer/data-access-objects/data-access-object-dynamodb.ts`:
- Around line 347-361: The IN branch (case FilterCriteriaEnum.in) must sanitize
and validate tokens before building the FilterExpression: trim and filter out
empty strings from the inValues array, and if isNumberField only keep tokens
that are valid numbers (reject NaN/empty), then only build placeholders, append
`AND #${field} IN (...)`, and set expressionAttributeValues for the remaining
valid tokens; if no valid tokens remain, skip adding this filter (do not emit
`IN ()`) or handle it as a no-op. Update the logic around uniquePlaceholder,
inValues, and expressionAttributeValues to use the filtered/validated array so
you never create empty IN lists or invalid { N: "" } values.
In
`@shared-code/src/data-access-layer/data-access-objects/data-access-object-ibmdb2.ts`:
- Around line 319-327: Detect empty inValues before building the IN clause in
the FilterCriteriaEnum.in branch: after computing inValues (from
filterObject.value) add a guard that if inValues.length === 0 returns a
always-false predicate (e.g., "1=0") and does not push any params to
queryParams; otherwise proceed to build placeholders and push params as
currently done. This change should be applied around the code using
FilterCriteriaEnum.in, filterObject, inValues, and queryParams so the DAO
(data-access-object-ibmdb2) never emits "IN ()".
In
`@shared-code/src/data-access-layer/data-access-objects/data-access-object-mysql.ts`:
- Around line 262-268: The in-list parsing currently leaves empty tokens (e.g.,
"active," → ["active",""]) before calling builder.whereIn, so update the code in
the DataAccessObjectMySQL handling of FilterCriteriaEnum.in (the block that
builds inValues and calls builder.whereIn) to trim and filter out empty strings
(e.g., map v => v.trim() then filter(v => v !== '') or filter(Boolean)), and if
the resulting inValues array is empty skip calling builder.whereIn; apply the
same fix to the duplicate stream-path occurrence that also builds inValues and
calls whereIn.
In
`@shared-code/src/data-access-layer/data-access-objects/data-access-object-oracle.ts`:
- Around line 813-822: The IN branch handling FilterCriteriaEnum.in currently
builds inValues without checking Oracle's 1000-item IN limit; update the code in
data-access-object-oracle.ts (where FilterCriteriaEnum.in is handled and
builder.whereIn is called) to validate inValues.length and prevent ORA-01795 by
either throwing a clear validation error when inValues.length > 1000 or
splitting inValues into chunks of ≤1000 and combining them into multiple IN
clauses (e.g., field IN (...) OR field IN (...)) via builder.where /
builder.orWhere so the query remains correct; ensure the change references the
existing variables builder, inValues and the FilterCriteriaEnum.in branch.
In
`@shared-code/src/data-access-layer/data-access-objects/data-access-object-redis.ts`:
- Around line 1852-1859: The in-case currently coerces non-array inputs using
String(value), which turns null/undefined into "null"/"undefined" and can cause
accidental matches; update the FilterCriteriaEnum.in branch (the inValues
variable computation) to first treat nullish inputs as an empty array (e.g., if
value == null then inValues = []), otherwise keep the existing
Array.isArray(value) branch and string-splitting fallback, and then use inValues
in the existing comparison against rowValue to set result; this ensures
null/undefined are not treated as searchable tokens while preserving current
behavior for arrays and comma-separated strings.
---
Nitpick comments:
In `@backend/src/entities/table/table-datastructures.ts`:
- Around line 15-16: The Swagger contract for FilteringFieldsDs.value is opaque
because it's typed as unknown with a bare `@ApiProperty`(); replace the bare
decorator with an explicit schema that documents the allowed single-value types
and the __in array form: update the `@ApiProperty` on the FilteringFieldsDs.value
field to use a oneOf that lists primitive single-value schemas (e.g., string,
number, boolean) and an array variant whose items use the same oneOf for the
supported primitives, so client generators see "single value OR array of values"
documented.
In `@backend/src/entities/table/utils/find-filtering-fields.util.ts`:
- Around line 114-125: parseFilteringFieldsFromBodyData currently pushes
filterData[key] directly (in the loop inside parseFilteringFieldsFromBodyData)
which fails to normalize "__in" values when clients pass a comma-separated
string; mirror the behavior in findFilteringFieldsUtil by detecting when key ===
FilterCriteriaEnum.in and normalizing the value before pushing: if the value is
a string, split on commas and trim into an array; if it's a non-array single
value, wrap it in an array; otherwise leave arrays as-is; then use that
normalized value in the filteringItems.push call (preserve the existing handling
for FilterCriteriaEnum.eq/empty).
In `@rocketadmin-agent/src/interfaces/interfaces.ts`:
- Around line 71-72: IMessageDataInfo currently uses `any` for `row` and
`primaryKey`; change it to a typed generic or structured unknown to restore type
safety (e.g., make the interface generic like IMessageDataInfo<TRow =
Record<string, unknown>, TKey = Record<string, unknown>> and set `row: TRow` and
`primaryKey: TKey`), and update callers to provide concrete type parameters or
accept the default Record<string, unknown>; ensure you update references to
IMessageDataInfo to pass types where needed so the compiler catches incorrect
message payload shapes.
In
`@shared-code/src/data-access-layer/data-access-objects/data-access-object-mssql.ts`:
- Around line 605-614: The IN-filter and general filter building code duplicated
in getRowsFromTable and getTableRowsStream should be extracted into a shared
helper (e.g., buildFilterClause or applyFilterToBuilder) that accepts the query
builder, field, criteria, value, operators, and values; move the logic that
handles FilterCriteriaEnum.in (normalizing inValues from arrays or comma strings
and calling builder.whereIn) and the fallback builder.where into that helper,
then replace the duplicated blocks in both getRowsFromTable and
getTableRowsStream with calls to the new helper so both methods use the same
centralized filter logic.
In
`@shared-code/src/data-access-layer/data-access-objects/data-access-object-postgres.ts`:
- Around line 673-682: The IN-filter branching in getTableRowsStream duplicates
the same logic found in getRowsFromTable; extract the shared filter application
into a new helper (e.g., applyFilters or buildWhereClauses) that accepts the
query builder and the filter definition(s) and handles FilterCriteriaEnum.in
(array/string comma-split) and the other criteria using operators[criteria] and
values[criteria] || value; then replace the duplicated blocks in
getTableRowsStream and getRowsFromTable with calls to that helper so both
functions delegate filter construction to the single utility.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 2b161c0f-7a69-4786-a9a2-42856d9148cb
📒 Files selected for processing (18)
backend/src/entities/table/application/data-structures/found-table-rows.ds.tsbackend/src/entities/table/table-datastructures.tsbackend/src/entities/table/utils/find-filtering-fields.util.tsbackend/src/enums/filter-criteria.enum.tsrocketadmin-agent/src/enums/filter-criteria.enum.tsrocketadmin-agent/src/interfaces/interfaces.tsshared-code/src/data-access-layer/data-access-objects/data-access-object-cassandra.tsshared-code/src/data-access-layer/data-access-objects/data-access-object-clickhouse.tsshared-code/src/data-access-layer/data-access-objects/data-access-object-dynamodb.tsshared-code/src/data-access-layer/data-access-objects/data-access-object-elasticsearch.tsshared-code/src/data-access-layer/data-access-objects/data-access-object-ibmdb2.tsshared-code/src/data-access-layer/data-access-objects/data-access-object-mongodb.tsshared-code/src/data-access-layer/data-access-objects/data-access-object-mssql.tsshared-code/src/data-access-layer/data-access-objects/data-access-object-mysql.tsshared-code/src/data-access-layer/data-access-objects/data-access-object-oracle.tsshared-code/src/data-access-layer/data-access-objects/data-access-object-postgres.tsshared-code/src/data-access-layer/data-access-objects/data-access-object-redis.tsshared-code/src/shared/enums/filter-criteria.enum.ts
Comment on lines
+1
to
+4
| import { AutocompleteFieldsDS } from '@rocketadmin/shared-code/dist/src/data-access-layer/shared/data-structures/autocomplete-fields.ds.js'; | ||
| import { FilteringFieldsDS } from '@rocketadmin/shared-code/dist/src/data-access-layer/shared/data-structures/filtering-fields.ds.js'; | ||
| import { TableSettingsDS } from '@rocketadmin/shared-code/dist/src/data-access-layer/shared/data-structures/table-settings.ds.js'; | ||
| import { TableStructureDS } from '@rocketadmin/shared-code/src/data-access-layer/shared/data-structures/table-structure.ds.js'; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify mixed import styles in agent code and inspect shared-code package exports.
rg -n "@rocketadmin/shared-code/(src|dist)/" rocketadmin-agent/src
if [ -f shared-code/package.json ]; then
echo "---- shared-code/package.json exports/main ----"
jq '{main, module, exports}' shared-code/package.json
fiRepository: rocket-admin/rocketadmin
Length of output: 2005
🏁 Script executed:
#!/bin/bash
# Check full interfaces.ts file size and retrieve lines around 71-72
wc -l rocketadmin-agent/src/interfaces/interfaces.ts
echo "--- Lines 1-10 of interfaces.ts ---"
head -10 rocketadmin-agent/src/interfaces/interfaces.ts
echo "--- Lines 65-80 of interfaces.ts (checking for any types) ---"
sed -n '65,80p' rocketadmin-agent/src/interfaces/interfaces.tsRepository: rocket-admin/rocketadmin
Length of output: 1460
🏁 Script executed:
#!/bin/bash
# Get shared-code package.json exports configuration
if [ -f shared-code/package.json ]; then
cat shared-code/package.json | head -50
fiRepository: rocket-admin/rocketadmin
Length of output: 1193
🏁 Script executed:
#!/bin/bash
# Check if /src path exists as a direct export or alias in shared-code
ls -la shared-code/ 2>/dev/null | head -20Repository: rocket-admin/rocketadmin
Length of output: 587
🏁 Script executed:
#!/bin/bash
# Get complete shared-code/package.json
cat shared-code/package.jsonRepository: rocket-admin/rocketadmin
Length of output: 1193
Fix import paths and add type safety to IMessageDataInfo.
Line 4 imports from @rocketadmin/shared-code/src/... while lines 1–3 import from @rocketadmin/shared-code/dist/src/.... Since no dist/ directory exists in shared-code and the package lacks export configuration, make all imports consistent using the direct source path pattern (e.g., @rocketadmin/shared-code/src/...).
Additionally, replace any types for row and primaryKey (lines 71–72 in IMessageDataInfo) with specific types to comply with TypeScript guidelines.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@rocketadmin-agent/src/interfaces/interfaces.ts` around lines 1 - 4, Update
the inconsistent import paths so all four imports use the source path pattern
(change the imports of AutocompleteFieldsDS, FilteringFieldsDS, TableSettingsDS
to import from '@rocketadmin/shared-code/src/...') to match TableStructureDS;
then in the IMessageDataInfo interface replace the loose any types on the
properties row and primaryKey with stricter types (use row: Record<string,
unknown> and primaryKey: string | number) to provide type safety and satisfy
TypeScript guidelines.
Comment on lines
+347
to
+361
| case FilterCriteriaEnum.in: { | ||
| const inValues = Array.isArray(value) | ||
| ? value | ||
| : String(value) | ||
| .split(',') | ||
| .map((v) => v.trim()); | ||
| const placeholders = inValues.map((_, i) => `${uniquePlaceholder}_${i}`); | ||
| filterExpression += ` AND #${field} IN (${placeholders.join(', ')})`; | ||
| inValues.forEach((val, i) => { | ||
| expressionAttributeValues[`${uniquePlaceholder}_${i}`] = isNumberField | ||
| ? { N: String(val) } | ||
| : { S: String(val) }; | ||
| }); | ||
| break; | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
Does DynamoDB FilterExpression allow an empty IN list (attribute IN ()), and what are the validation rules for numeric AttributeValues ({ "N": "..." })?
💡 Result:
No, DynamoDB FilterExpression does not allow an empty IN list like attribute IN . The IN operator syntax requires operand IN (operand (, ...)) with at least one operand, up to 100 values. An empty list is syntactically invalid and will cause a ValidationException. Use alternatives like attribute_not_exists(attribute) or NOT attribute_exists(attribute) for no-match conditions, or size(list_attribute) = :zero where :zero = {"N": "0"} for empty lists/sets. For numeric AttributeValues {"N": "..."}, the string must represent a valid number: positive/negative/zero, up to 38 digits precision. Range: 1E-130 to ~9.999E+125 positive, symmetric negative. Leading/trailing zeros trimmed. Invalid formats (non-numeric strings, >38 digits) cause ValidationException.
Citations:
- 1: https://docs.aws.amazon.com/amazondynamodb/latest/APIReference/API_AttributeValue.html
- 2: https://docs.aws.amazon.com/amazondynamodb/latest/developerguide/HowItWorks.NamingRulesDataTypes.html
- 3: https://docs.aws.amazon.com/amazondynamodb/latest/developerguide/Expressions.OperatorsAndFunctions.html
- 4: https://docs.aws.amazon.com/amazondynamodb/latest/APIReference/API_Condition.html
- 5: https://stackoverflow.com/questions/39576558/filter-dynamodb-query-using-in
- 6: https://docs.aws.amazon.com/amazondynamodb/latest/developerguide/example_dynamodb_Scenario_CompareMultipleValues_section.html
- 7: https://stackoverflow.com/questions/32671509/in-statement-in-dynamodb
- 8: https://stackoverflow.com/questions/41817735/dynamodb-filterexpression-with-not-in
- 9: https://oneuptime.com/blog/post/2026-02-12-fix-dynamodb-validationexception-errors/view
🏁 Script executed:
# Find and read the file
fd -p "data-access-object-dynamodb.ts" shared-code/src/data-access-layer/Repository: rocket-admin/rocketadmin
Length of output: 152
🏁 Script executed:
# Read the specific lines around 347-361
sed -n '340,370p' shared-code/src/data-access-layer/data-access-objects/data-access-object-dynamodb.tsRepository: rocket-admin/rocketadmin
Length of output: 1046
🏁 Script executed:
# Get broader context of the method containing this code
sed -n '320,380p' shared-code/src/data-access-layer/data-access-objects/data-access-object-dynamodb.tsRepository: rocket-admin/rocketadmin
Length of output: 2308
🏁 Script executed:
# Check the function signature and broader error handling context
sed -n '300,340p' shared-code/src/data-access-layer/data-access-objects/data-access-object-dynamodb.tsRepository: rocket-admin/rocketadmin
Length of output: 2160
🏁 Script executed:
# Search for error handling patterns in similar filter construction
rg "throw new Error|ValidationException|invalid" shared-code/src/data-access-layer/data-access-objects/data-access-object-dynamodb.ts -B 2 -A 2Repository: rocket-admin/rocketadmin
Length of output: 479
Guard DynamoDB IN construction against empty and invalid numeric values.
The current code can produce invalid FilterExpressions and AttributeValues:
- If
valueis[]or splits to empty tokens (e.g.,"1,"), the code generatesIN ()syntax, which DynamoDB rejects with a ValidationException. - Non-numeric strings passed to numeric fields create invalid
{ "N": "" }or{ "N": "abc" }, also causing ValidationException.
Sanitize tokens and validate numeric entries before building the expression and placeholders.
Suggested fix
case FilterCriteriaEnum.in: {
- const inValues = Array.isArray(value)
- ? value
- : String(value)
- .split(',')
- .map((v) => v.trim());
+ const inValues = (Array.isArray(value) ? value : String(value).split(','))
+ .map((v) => String(v).trim())
+ .filter((v) => v.length > 0);
+
+ if (inValues.length === 0) {
+ filterExpression += ` AND attribute_exists(#${field}) AND attribute_not_exists(#${field})`;
+ break;
+ }
+
+ if (isNumberField && inValues.some((val) => Number.isNaN(Number(val)))) {
+ throw new Error(`Invalid numeric value in IN filter for field "${field}"`);
+ }
+
const placeholders = inValues.map((_, i) => `${uniquePlaceholder}_${i}`);
filterExpression += ` AND #${field} IN (${placeholders.join(', ')})`;
inValues.forEach((val, i) => {
expressionAttributeValues[`${uniquePlaceholder}_${i}`] = isNumberField
? { N: String(val) }
: { S: String(val) };
});
break;
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| case FilterCriteriaEnum.in: { | |
| const inValues = Array.isArray(value) | |
| ? value | |
| : String(value) | |
| .split(',') | |
| .map((v) => v.trim()); | |
| const placeholders = inValues.map((_, i) => `${uniquePlaceholder}_${i}`); | |
| filterExpression += ` AND #${field} IN (${placeholders.join(', ')})`; | |
| inValues.forEach((val, i) => { | |
| expressionAttributeValues[`${uniquePlaceholder}_${i}`] = isNumberField | |
| ? { N: String(val) } | |
| : { S: String(val) }; | |
| }); | |
| break; | |
| } | |
| case FilterCriteriaEnum.in: { | |
| const inValues = (Array.isArray(value) ? value : String(value).split(',')) | |
| .map((v) => String(v).trim()) | |
| .filter((v) => v.length > 0); | |
| if (inValues.length === 0) { | |
| filterExpression += ` AND attribute_exists(#${field}) AND attribute_not_exists(#${field})`; | |
| break; | |
| } | |
| if (isNumberField && inValues.some((val) => Number.isNaN(Number(val)))) { | |
| throw new Error(`Invalid numeric value in IN filter for field "${field}"`); | |
| } | |
| const placeholders = inValues.map((_, i) => `${uniquePlaceholder}_${i}`); | |
| filterExpression += ` AND #${field} IN (${placeholders.join(', ')})`; | |
| inValues.forEach((val, i) => { | |
| expressionAttributeValues[`${uniquePlaceholder}_${i}`] = isNumberField | |
| ? { N: String(val) } | |
| : { S: String(val) }; | |
| }); | |
| break; | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@shared-code/src/data-access-layer/data-access-objects/data-access-object-dynamodb.ts`
around lines 347 - 361, The IN branch (case FilterCriteriaEnum.in) must sanitize
and validate tokens before building the FilterExpression: trim and filter out
empty strings from the inValues array, and if isNumberField only keep tokens
that are valid numbers (reject NaN/empty), then only build placeholders, append
`AND #${field} IN (...)`, and set expressionAttributeValues for the remaining
valid tokens; if no valid tokens remain, skip adding this filter (do not emit
`IN ()`) or handle it as a no-op. Update the logic around uniquePlaceholder,
inValues, and expressionAttributeValues to use the filtered/validated array so
you never create empty IN lists or invalid { N: "" } values.
Comment on lines
+319
to
+327
| case FilterCriteriaEnum.in: { | ||
| const inValues = Array.isArray(filterObject.value) | ||
| ? filterObject.value | ||
| : String(filterObject.value) | ||
| .split(',') | ||
| .map((v) => v.trim()); | ||
| const placeholders = inValues.map(() => '?').join(', '); | ||
| queryParams.push(...(inValues as SQLParam[])); | ||
| return `${filterObject.field} IN (${placeholders})`; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check IN-filter handling across DAO implementations and parser normalization.
rg -nP --type=ts 'case\s+FilterCriteriaEnum\.in' shared-code/src/data-access-layer/data-access-objects
rg -nP --type=ts '__in|FilterCriteriaEnum\.in' backend/src/entities/table/utils
rg -nP --type=ts 'split\(\x27,\x27\)|IN \(\$\{placeholders\}\)' shared-code/src/data-access-layer/data-access-objectsRepository: rocket-admin/rocketadmin
Length of output: 3655
🏁 Script executed:
# First, check the exact IBM DB2 implementation at lines 319-327
sed -n '310,335p' shared-code/src/data-access-layer/data-access-objects/data-access-object-ibmdb2.tsRepository: rocket-admin/rocketadmin
Length of output: 1035
🏁 Script executed:
# Check other DAO implementations for similar patterns and any empty guards
# MongoDB at line 278
sed -n '270,300p' shared-code/src/data-access-layer/data-access-objects/data-access-object-mongodb.tsRepository: rocket-admin/rocketadmin
Length of output: 954
🏁 Script executed:
# Check Postgres implementation
sed -n '220,250p' shared-code/src/data-access-layer/data-access-objects/data-access-object-postgres.tsRepository: rocket-admin/rocketadmin
Length of output: 1039
🏁 Script executed:
# Check the parser logic for empty value handling
sed -n '85,110p' backend/src/entities/table/utils/find-filtering-fields.util.tsRepository: rocket-admin/rocketadmin
Length of output: 718
🏁 Script executed:
# Check if there's any validation after the split pattern in any DAO
rg -A 3 'FilterCriteriaEnum\.in' shared-code/src/data-access-layer/data-access-objects/data-access-object-postgres.ts | head -30Repository: rocket-admin/rocketadmin
Length of output: 336
🏁 Script executed:
# Check Clickhouse implementation
sed -n '260,280p' shared-code/src/data-access-layer/data-access-objects/data-access-object-clickhouse.tsRepository: rocket-admin/rocketadmin
Length of output: 724
🏁 Script executed:
# Check if there are any guards or validation patterns for empty filters
rg -B 2 -A 5 'inValues\.length|inValues === \[\]|\.filter' shared-code/src/data-access-layer/data-access-objects/ | head -40Repository: rocket-admin/rocketadmin
Length of output: 4306
🏁 Script executed:
# Check if there's input validation in the parser or upstream
rg -B 5 'rawValue|filteringItems.push' backend/src/entities/table/utils/find-filtering-fields.util.ts | head -50Repository: rocket-admin/rocketadmin
Length of output: 1387
🏁 Script executed:
# Check if there are any tests that cover empty IN scenarios
find . -type f -name "*.spec.ts" -o -name "*.test.ts" | xargs grep -l "FilterCriteriaEnum.in" 2>/dev/null | head -5Repository: rocket-admin/rocketadmin
Length of output: 50
🏁 Script executed:
# Check if there's any actual SQL execution that shows whether empty IN causes failures
rg -B 5 -A 10 'case FilterCriteriaEnum\.in' shared-code/src/data-access-layer/data-access-objects/data-access-object-ibmdb2.tsRepository: rocket-admin/rocketadmin
Length of output: 704
🏁 Script executed:
# Check if Knex's whereIn has any documented empty array handling behavior (look for comments or type hints)
rg -B 3 -A 3 'whereIn' shared-code/src/data-access-layer/data-access-objects/data-access-object-postgres.ts | head -20Repository: rocket-admin/rocketadmin
Length of output: 602
🏁 Script executed:
# Verify the exact line 327 code to confirm the issue
sed -n '319,330p' shared-code/src/data-access-layer/data-access-objects/data-access-object-ibmdb2.tsRepository: rocket-admin/rocketadmin
Length of output: 482
🏁 Script executed:
# Final check: Verify if the fix is correct by looking at context - does the code reach this point with empty arrays?
sed -n '310,340p' shared-code/src/data-access-layer/data-access-objects/data-access-object-ibmdb2.tsRepository: rocket-admin/rocketadmin
Length of output: 1293
🏁 Script executed:
# Check if any downstream error handling catches invalid SQL or if it fails hard
rg -B 3 -A 3 'queryParams\|queryString|execute\|query\(' shared-code/src/data-access-layer/data-access-objects/data-access-object-ibmdb2.ts | grep -A 5 -B 5 'buildWhere'Repository: rocket-admin/rocketadmin
Length of output: 50
Guard against empty IN lists to prevent invalid SQL syntax.
When filterObject.value is empty (e.g., empty array or empty/whitespace-only string), this generates invalid SQL IN () and causes query failure. This issue affects multiple SQL-based DAOs (IBM DB2, PostgreSQL, MySQL, MSSQL, Oracle, ClickHouse, Cassandra). Add an explicit empty-list guard before building the IN clause.
Suggested guard implementation
case FilterCriteriaEnum.in: {
- const inValues = Array.isArray(filterObject.value)
+ const rawValues = Array.isArray(filterObject.value)
? filterObject.value
: String(filterObject.value)
.split(',')
.map((v) => v.trim());
- const placeholders = inValues.map(() => '?').join(', ');
+ const inValues = rawValues.filter((v) => String(v).length > 0);
+ if (inValues.length === 0) {
+ return '1 = 0';
+ }
+ const placeholders = inValues.map(() => '?').join(', ');
queryParams.push(...(inValues as SQLParam[]));
return `${filterObject.field} IN (${placeholders})`;
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| case FilterCriteriaEnum.in: { | |
| const inValues = Array.isArray(filterObject.value) | |
| ? filterObject.value | |
| : String(filterObject.value) | |
| .split(',') | |
| .map((v) => v.trim()); | |
| const placeholders = inValues.map(() => '?').join(', '); | |
| queryParams.push(...(inValues as SQLParam[])); | |
| return `${filterObject.field} IN (${placeholders})`; | |
| case FilterCriteriaEnum.in: { | |
| const rawValues = Array.isArray(filterObject.value) | |
| ? filterObject.value | |
| : String(filterObject.value) | |
| .split(',') | |
| .map((v) => v.trim()); | |
| const inValues = rawValues.filter((v) => String(v).length > 0); | |
| if (inValues.length === 0) { | |
| return '1 = 0'; | |
| } | |
| const placeholders = inValues.map(() => '?').join(', '); | |
| queryParams.push(...(inValues as SQLParam[])); | |
| return `${filterObject.field} IN (${placeholders})`; | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@shared-code/src/data-access-layer/data-access-objects/data-access-object-ibmdb2.ts`
around lines 319 - 327, Detect empty inValues before building the IN clause in
the FilterCriteriaEnum.in branch: after computing inValues (from
filterObject.value) add a guard that if inValues.length === 0 returns a
always-false predicate (e.g., "1=0") and does not push any params to
queryParams; otherwise proceed to build placeholders and push params as
currently done. This change should be applied around the code using
FilterCriteriaEnum.in, filterObject, inValues, and queryParams so the DAO
(data-access-object-ibmdb2) never emits "IN ()".
Comment on lines
+262
to
+268
| if (criteria === FilterCriteriaEnum.in) { | ||
| const inValues = Array.isArray(value) | ||
| ? value | ||
| : String(value) | ||
| .split(',') | ||
| .map((v) => v.trim()); | ||
| builder.whereIn(field, inValues); |
There was a problem hiding this comment.
Filter out empty __in tokens before applying whereIn.
Current parsing keeps empty values (e.g., "active," → ["active", ""]), which can unintentionally match empty-string rows. This affects both paged and stream paths.
Suggested fix
-const inValues = Array.isArray(value)
- ? value
- : String(value)
- .split(',')
- .map((v) => v.trim());
-builder.whereIn(field, inValues);
+const inValues = (Array.isArray(value) ? value : String(value).split(','))
+ .map((v) => String(v).trim())
+ .filter((v) => v.length > 0);
+
+if (inValues.length === 0) {
+ builder.whereRaw('1 = 0');
+} else {
+ builder.whereIn(field, inValues);
+}Also applies to: 653-659
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@shared-code/src/data-access-layer/data-access-objects/data-access-object-mysql.ts`
around lines 262 - 268, The in-list parsing currently leaves empty tokens (e.g.,
"active," → ["active",""]) before calling builder.whereIn, so update the code in
the DataAccessObjectMySQL handling of FilterCriteriaEnum.in (the block that
builds inValues and calls builder.whereIn) to trim and filter out empty strings
(e.g., map v => v.trim() then filter(v => v !== '') or filter(Boolean)), and if
the resulting inValues array is empty skip calling builder.whereIn; apply the
same fix to the duplicate stream-path occurrence that also builds inValues and
calls whereIn.
Comment on lines
+813
to
+822
| if (criteria === FilterCriteriaEnum.in) { | ||
| const inValues = Array.isArray(value) | ||
| ? value | ||
| : String(value) | ||
| .split(',') | ||
| .map((v) => v.trim()); | ||
| builder.whereIn(field, inValues); | ||
| } else { | ||
| builder.where(field, operators[criteria], values[criteria] || value); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
Oracle IN clause maximum number of values limit ORA-01795
💡 Result:
The Oracle IN clause has a maximum limit of 1000 expressions (literals or values) in a list, exceeding which triggers ORA-01795: maximum number of expressions in a list is 1000. This limit applies across Oracle versions, including the latest (26ai as of 2026), as confirmed by official Oracle documentation. Note that unused expressions in the query also count toward this limit. Workarounds include: - Split the list into multiple IN clauses combined with OR (e.g., IN (1..1000) OR IN (1001..2000)). - Use tuples for higher limits: WHERE (constant, column) IN ((constant, val1), (constant, val2), ...). - Load values into a temporary table or collection (e.g., sys.odcinumberlist) and join/subquery against it. - Use UNION ALL for separate IN clauses.
Citations:
- 1: https://docs.oracle.com/en/error-help/db/ora-01795/index.html?r=23ai
- 2: https://dbaparadise.com/2016/07/limitations-of-the-in-clause-in-oracle-ora-01795/
- 3: https://stackoverflow.com/questions/17842453/is-there-a-workaround-for-ora-01795-maximum-number-of-expressions-in-a-list-is
- 4: https://docs.oracle.com/en/database/oracle/oracle-database/23/refrn/logical-database-limits.html
🏁 Script executed:
#!/bin/bash
# First, check the file size to determine how to read it
wc -l shared-code/src/data-access-layer/data-access-objects/data-access-object-oracle.ts
# Get context around lines 813-822
sed -n '800,835p' shared-code/src/data-access-layer/data-access-objects/data-access-object-oracle.tsRepository: rocket-admin/rocketadmin
Length of output: 1216
🏁 Script executed:
#!/bin/bash
# Search for any validation or batching logic related to array/value limits
rg -n "length|limit|batch|chunk|split.*1000|MAX_" shared-code/src/data-access-layer/data-access-objects/data-access-object-oracle.ts -A 3 -B 3
# Search for other whereIn usages in this file
rg -n "whereIn" shared-code/src/data-access-layer/data-access-objects/data-access-object-oracle.tsRepository: rocket-admin/rocketadmin
Length of output: 4843
🏁 Script executed:
#!/bin/bash
# Check if there are any similar patterns in other data access layer files
fd -e ts shared-code/src/data-access-layer/ --exec rg -l "whereIn" {}Repository: rocket-admin/rocketadmin
Length of output: 509
🏁 Script executed:
#!/bin/bash
# Get a larger context around the whereIn call (lines 790-835)
sed -n '790,835p' shared-code/src/data-access-layer/data-access-objects/data-access-object-oracle.tsRepository: rocket-admin/rocketadmin
Length of output: 1547
🏁 Script executed:
#!/bin/bash
# Search for any existing validation, error handling, or warnings about Oracle limits
rg -i "oracle|1000|limit|batch" shared-code/src/data-access-layer/data-access-objects/data-access-object-oracle.ts | head -20Repository: rocket-admin/rocketadmin
Length of output: 1113
🏁 Script executed:
#!/bin/bash
# Check if there are any comments or documentation about limitations
rg -n "ORA-|limit|maximum" shared-code/src/data-access-layer/data-access-objects/data-access-object-oracle.ts -B 2 -A 2Repository: rocket-admin/rocketadmin
Length of output: 670
🏁 Script executed:
#!/bin/bash
# Search for whereIn usage across the entire data access layer
fd --full-path 'shared-code/src/data-access-layer' -type f -name '*.ts' -exec rg -l "whereIn" {} \;Repository: rocket-admin/rocketadmin
Length of output: 238
🏁 Script executed:
#!/bin/bash
# Correct fd syntax to search in directory
fd -type f -name '*.ts' 'shared-code/src/data-access-layer' -exec rg -l "whereIn" {} \;Repository: rocket-admin/rocketadmin
Length of output: 238
🏁 Script executed:
#!/bin/bash
# Check the other whereIn call at line 344 for comparison
sed -n '330,360p' shared-code/src/data-access-layer/data-access-objects/data-access-object-oracle.tsRepository: rocket-admin/rocketadmin
Length of output: 968
🏁 Script executed:
#!/bin/bash
# Search for any documentation or comments about Oracle limitations
rg -n "Oracle\|ORA-\|IN.*clause\|1000" shared-code/src/data-access-layer/ -A 2 -B 2Repository: rocket-admin/rocketadmin
Length of output: 50
🏁 Script executed:
#!/bin/bash
# Simpler approach - just find the directory and check files
find shared-code/src/data-access-layer -name '*.ts' -type f | head -20Repository: rocket-admin/rocketadmin
Length of output: 1727
🏁 Script executed:
#!/bin/bash
# Check if there's any validation on filter values before they reach the DAO
find shared-code/src -name '*.ts' -type f | xargs rg -l "FilterCriteriaEnum.in|whereIn" | head -10Repository: rocket-admin/rocketadmin
Length of output: 910
🏁 Script executed:
#!/bin/bash
# Look for any comments or warnings about Oracle IN clause limitations
find shared-code -name '*.ts' -o -name '*.md' | xargs rg -i "in.*clause|value.*limit|1000.*value" 2>/dev/nullRepository: rocket-admin/rocketadmin
Length of output: 228
🏁 Script executed:
#!/bin/bash
# Check if other database-specific DAOs have any safeguards for large arrays
rg -n "whereIn" shared-code/src/data-access-layer/data-access-objects/data-access-object-mysql.ts -B 5 -A 5 | head -30Repository: rocket-admin/rocketadmin
Length of output: 1125
🏁 Script executed:
#!/bin/bash
# Check what FilterCriteriaEnum.in is used for - understand the use case
find shared-code -name '*.ts' -type f | xargs rg -l "FilterCriteriaEnum" | head -5Repository: rocket-admin/rocketadmin
Length of output: 451
🏁 Script executed:
#!/bin/bash
# Look for filtering-fields.ds.ts to understand where filter values come from
cat shared-code/src/data-access-layer/shared/data-structures/filtering-fields.ds.tsRepository: rocket-admin/rocketadmin
Length of output: 251
Add validation for Oracle IN clause 1000-value limit.
The Oracle IN clause is limited to 1000 values; exceeding this triggers ORA-01795. The code at lines 813-822 accepts user-provided filter arrays without validation. Consider adding a check to validate inValues.length and either throw an error or batch the values into multiple IN clauses.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@shared-code/src/data-access-layer/data-access-objects/data-access-object-oracle.ts`
around lines 813 - 822, The IN branch handling FilterCriteriaEnum.in currently
builds inValues without checking Oracle's 1000-item IN limit; update the code in
data-access-object-oracle.ts (where FilterCriteriaEnum.in is handled and
builder.whereIn is called) to validate inValues.length and prevent ORA-01795 by
either throwing a clear validation error when inValues.length > 1000 or
splitting inValues into chunks of ≤1000 and combining them into multiple IN
clauses (e.g., field IN (...) OR field IN (...)) via builder.where /
builder.orWhere so the query remains correct; ensure the change references the
existing variables builder, inValues and the FilterCriteriaEnum.in branch.
Comment on lines
+1852
to
+1859
| case FilterCriteriaEnum.in: { | ||
| const inValues = Array.isArray(value) | ||
| ? value | ||
| : String(value) | ||
| .split(',') | ||
| .map((v) => v.trim()); | ||
| result = inValues.some((v) => String(rowValue) === String(v)); | ||
| break; |
There was a problem hiding this comment.
Defensively normalize nullable in values to avoid accidental matches.
Coercing non-arrays with String(value) turns null/undefined into searchable tokens. Prefer treating nullish inputs as an empty candidate list.
Defensive normalization tweak
case FilterCriteriaEnum.in: {
- const inValues = Array.isArray(value)
- ? value
- : String(value)
+ const rawValues = Array.isArray(value)
+ ? value
+ : value == null
+ ? []
+ : String(value)
.split(',')
.map((v) => v.trim());
- result = inValues.some((v) => String(rowValue) === String(v));
+ const inValues = rawValues.filter((v) => String(v).length > 0);
+ result = inValues.length > 0 && inValues.some((v) => String(rowValue) === String(v));
break;
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| case FilterCriteriaEnum.in: { | |
| const inValues = Array.isArray(value) | |
| ? value | |
| : String(value) | |
| .split(',') | |
| .map((v) => v.trim()); | |
| result = inValues.some((v) => String(rowValue) === String(v)); | |
| break; | |
| case FilterCriteriaEnum.in: { | |
| const rawValues = Array.isArray(value) | |
| ? value | |
| : value == null | |
| ? [] | |
| : String(value) | |
| .split(',') | |
| .map((v) => v.trim()); | |
| const inValues = rawValues.filter((v) => String(v).length > 0); | |
| result = inValues.length > 0 && inValues.some((v) => String(rowValue) === String(v)); | |
| break; |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@shared-code/src/data-access-layer/data-access-objects/data-access-object-redis.ts`
around lines 1852 - 1859, The in-case currently coerces non-array inputs using
String(value), which turns null/undefined into "null"/"undefined" and can cause
accidental matches; update the FilterCriteriaEnum.in branch (the inValues
variable computation) to first treat nullish inputs as an empty array (e.g., if
value == null then inValues = []), otherwise keep the existing
Array.isArray(value) branch and string-splitting fallback, and then use inValues
in the existing comparison against rowValue to set result; this ensures
null/undefined are not treated as searchable tokens while preserving current
behavior for arrays and comma-separated strings.
Artuomka
approved these changes
Apr 3, 2026
…ivers Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Artuomka
approved these changes
Apr 3, 2026
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
shared-code/src/data-access-layer/data-access-objects/data-access-object-oracle.ts (1)
312-318:⚠️ Potential issue | 🟠 MajorAvoid scalar date/timestamp conversion for array-based filters.
Line 312-318 coerces
valueusingString(value)before Line 338+ handlesin/between. For date/timestamp fields with array filters, this corrupts the input.Suggested fix
- if (datesColumnsNames.includes(field) && value) { + if (datesColumnsNames.includes(field) && value && !Array.isArray(value)) { const valueToDate = new Date(String(value)); value = this.formatDate(valueToDate); } - if (timestampColumnNames.includes(field) && value) { + if (timestampColumnNames.includes(field) && value && !Array.isArray(value)) { value = this.formatTimestamp(String(value)); }Also applies to: 338-345
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@shared-code/src/data-access-layer/data-access-objects/data-access-object-oracle.ts` around lines 312 - 318, The code in data-access-object-oracle.ts is coercing date/timestamp filter values with String(value) and converting them via formatDate/formatTimestamp unconditionally, which corrupts array-based filters (IN/BETWEEN); update the logic in the block that checks datesColumnsNames and timestampColumnNames so it first detects Array.isArray(value) and either map each element through formatDate/formatTimestamp (preserving arrays) or skip scalar coercion so the later IN/BETWEEN handling can format elements correctly; reference the existing symbols datesColumnsNames, timestampColumnNames, formatDate, formatTimestamp and ensure the same array-aware change is applied to the similar conversion code near the IN/BETWEEN handling.
♻️ Duplicate comments (1)
shared-code/src/data-access-layer/data-access-objects/data-access-object-ibmdb2.ts (1)
319-337:⚠️ Potential issue | 🟠 MajorGuard
IN/BETWEENinputs before generating SQL.Line 319-337 can still emit invalid SQL (
IN ()) or bindundefinedforBETWEENupper bound when input is malformed.Suggested fix
case FilterCriteriaEnum.in: { - const inValues = Array.isArray(filterObject.value) + const inValues = (Array.isArray(filterObject.value) ? filterObject.value : String(filterObject.value) .split(',') - .map((v) => v.trim()); + .map((v) => v.trim())) + .filter((v) => String(v).length > 0); + if (inValues.length === 0) { + return '1 = 0'; + } const placeholders = inValues.map(() => '?').join(', '); queryParams.push(...(inValues as SQLParam[])); return `${filterObject.field} IN (${placeholders})`; } case FilterCriteriaEnum.between: { - const betweenValues = Array.isArray(filterObject.value) + const betweenValues = (Array.isArray(filterObject.value) ? filterObject.value : String(filterObject.value) .split(',') - .map((v) => v.trim()); + .map((v) => v.trim())) + .filter((v) => String(v).length > 0); + if (betweenValues.length !== 2) { + throw new Error(`Invalid between filter for "${filterObject.field}"`); + } queryParams.push(betweenValues[0] as SQLParam, betweenValues[1] as SQLParam); return `${filterObject.field} BETWEEN ? AND ?`; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@shared-code/src/data-access-layer/data-access-objects/data-access-object-ibmdb2.ts` around lines 319 - 337, Guard the IN and BETWEEN branches in data-access-object-ibmdb2.ts to avoid emitting invalid SQL or binding undefined: in the FilterCriteriaEnum.in case (the in branch) validate inValues is a non-empty array before creating placeholders and pushing to queryParams—if empty, return a safe false predicate (e.g., "1=0") or skip the filter; in the FilterCriteriaEnum.between case validate there are at least two values (or a defined low and high) before pushing to queryParams and returning "field BETWEEN ? AND ?"; only push parameters when the corresponding values exist and handle malformed input by returning a safe predicate or rejecting the filter so no "IN ()" or undefined binds are produced.
🧹 Nitpick comments (5)
shared-code/src/data-access-layer/data-access-objects/data-access-object-clickhouse.ts (1)
3-3: Change import to use the package root public exports.Line 3 imports
NodeClickHouseClientConfigOptionsfrom@clickhouse/client/dist/config.js, which is not an officially supported API surface. For@clickhouse/clientv1.18.2, import from the package root instead:import type { NodeClickHouseClientConfigOptions } from '@clickhouse/client';This ensures compatibility with the package's public API contract and avoids potential breakage with bundlers, TypeScript resolution, or version updates.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@shared-code/src/data-access-layer/data-access-objects/data-access-object-clickhouse.ts` at line 3, The import of NodeClickHouseClientConfigOptions is using an internal path; replace the current import of NodeClickHouseClientConfigOptions from '@clickhouse/client/dist/config.js' with a package-root, type-only import (import type { NodeClickHouseClientConfigOptions } from '@clickhouse/client') so the code uses the public API surface; update the import statement in the module that references NodeClickHouseClientConfigOptions (e.g., any usage in the data-access-object-clickhouse module) to the package-root export.backend/test/ava-tests/non-saas-tests/non-saas-table-mysql-e2e.test.ts (4)
3669-3701: Missing try/catch error handling for consistency with other tests.All other tests in this file wrap their logic in try/catch blocks with
console.error(e); throw e;to aid debugging. The new IN/BETWEEN filter tests should follow the same pattern for consistency.♻️ Suggested refactor to add error handling
test.serial(`${currentTest} should return rows filtered by IN operator in body`, async (t) => { + try { const connectionToTestDB = getTestData(mockFactory).connectionToMySQL; const firstUserToken = (await registerUserAndReturnUserInfo(app)).token; const { testTableName } = await createTestTable(connectionToTestDB); testTables.push(testTableName); const createConnectionResponse = await request(app.getHttpServer()) .post('/connection') .send(connectionToTestDB) .set('Cookie', firstUserToken) .set('Content-Type', 'application/json') .set('Accept', 'application/json'); const createConnectionRO = JSON.parse(createConnectionResponse.text); t.is(createConnectionResponse.status, 201); const filters = { id: { in: ['1', '22', '38'] }, }; const getTableRowsResponse = await request(app.getHttpServer()) .post(`/table/rows/find/${createConnectionRO.id}?tableName=${testTableName}&page=1&perPage=20`) .send({ filters }) .set('Cookie', firstUserToken) .set('Content-Type', 'application/json') .set('Accept', 'application/json'); const getTableRowsRO = JSON.parse(getTableRowsResponse.text); t.is(getTableRowsResponse.status, 200); t.is(getTableRowsRO.rows.length, 3); const returnedIds = getTableRowsRO.rows.map((row) => row.id).sort((a, b) => a - b); t.deepEqual(returnedIds, [1, 22, 38]); + } catch (e) { + console.error(e); + throw e; + } });As per coding guidelines, "Ensure all error handling is explicit - use try/catch blocks appropriately".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/test/ava-tests/non-saas-tests/non-saas-table-mysql-e2e.test.ts` around lines 3669 - 3701, The new test case in test.serial(`${currentTest} should return rows filtered by IN operator in body`, async (t) => { ... }) is missing the try/catch wrapper used by other tests; wrap the entire async test body in a try { ... } catch (e) { console.error(e); throw e; } block so errors are logged and rethrown for consistency (locate the test by the test.serial call and the `currentTest`-based title).
3766-3793: Same try/catch pattern should be applied.This test also needs try/catch wrapping for consistency.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/test/ava-tests/non-saas-tests/non-saas-table-mysql-e2e.test.ts` around lines 3766 - 3793, The test body for test.serial(`${currentTest} should return rows filtered by BETWEEN operator in query params`) lacks the try/catch wrapper used elsewhere; wrap the core async test logic (everything from obtaining connectionToTestDB through assertions) in a try { ... } catch (err) { t.fail(err as Error) } block (or t.fail(String(err))) so failures are reported consistently, keep the existing testTables.push(testTableName) placement (or move it inside try before operations) and ensure the response parsing/assertions (getTableRowsResponse, t.is, t.deepEqual) remain inside the try block so errors are caught and handled the same way as other tests.
3703-3730: Same try/catch pattern should be applied.This test also needs try/catch wrapping for consistency with the rest of the test file.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/test/ava-tests/non-saas-tests/non-saas-table-mysql-e2e.test.ts` around lines 3703 - 3730, Wrap the body of the test.serial block (the test that checks IN operator filtering) in a try/catch similar to other tests: put the existing logic (calls to registerUserAndReturnUserInfo, createTestTable, request calls and assertions) inside a try, and in the catch block call t.fail(error) (or t.fail(String(error))) to surface the failure; reference the test.serial block and related helpers registerUserAndReturnUserInfo and createTestTable to locate where to add the try/catch.
3732-3764: Same try/catch pattern should be applied.This test also needs try/catch wrapping for consistency.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/test/ava-tests/non-saas-tests/non-saas-table-mysql-e2e.test.ts` around lines 3732 - 3764, The test block named `${currentTest} should return rows filtered by BETWEEN operator in body` currently lacks the try/catch wrapper used elsewhere; wrap the entire async test body (the calls to registerUserAndReturnUserInfo, createTestTable, the POST /connection request and the POST /table/rows/find request and assertions) in a try { ... } catch (err) { t.fail(err); } (or t.log(err); t.fail(err);) so any runtime errors are reported consistently — locate the test by the test.serial(...) declaration and the helper calls registerUserAndReturnUserInfo and createTestTable to find where to apply the wrapper.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@backend/src/entities/table/utils/find-filtering-fields.util.ts`:
- Around line 128-129: Guard against non-object payloads before iterating
filtersDataFromBody[rowName]: ensure the value (assigned to filterData) is a
non-null plain object (typeof === 'object' && filterData !== null &&
!Array.isArray(filterData)) and bail/continue if not, so iteration over
filterData in the loop does not throw or produce invalid criteria; apply this
check around where filtersDataFromBody, rowName, and filterData are used (in the
function in find-filtering-fields.util.ts) and handle invalid shapes by skipping
or returning an appropriate default.
- Around line 91-115: The current handling of filters[`f_${fieldname}__in`] and
filters[`f_${fieldname}__between`] can produce empty tokens or wrong arity;
before pushing into filteringItems (for FilterCriteriaEnum.in and
FilterCriteriaEnum.between) validate and normalize rawValue: convert non-array
to String(...).split(',').map(v=>v.trim()).filter(Boolean) and for __in only
push if the resulting array.length > 0; for __between ensure the resulting
array.length === 2 and both values are non-empty (or otherwise skip/throw),
using the existing symbols filters, fieldname, filteringItems,
FilterCriteriaEnum.in and FilterCriteriaEnum.between to locate and update the
logic.
In
`@shared-code/src/data-access-layer/data-access-objects/data-access-object-clickhouse.ts`:
- Around line 264-283: The IN and BETWEEN branches in the switch (handling
FilterCriteriaEnum.in and FilterCriteriaEnum.between) currently build SQL
without validating inputs; update these branches in
data-access-object-clickhouse.ts to first normalize the incoming value into an
array, then validate: for FilterCriteriaEnum.in ensure the resulting inValues
array is non-empty (otherwise throw a clear error or skip adding the where
clause) and only then map through this.sanitizeValue and push `${escapedField}
IN (...)`; for FilterCriteriaEnum.between ensure betweenValues has at least two
non-empty elements (otherwise throw/skip) and use sanitized betweenValues[0] and
betweenValues[1] when pushing the `${escapedField} BETWEEN ... AND ...` clause;
keep using the existing this.sanitizeValue, escapedField, and whereClauses
variables to locate the code to change.
In
`@shared-code/src/data-access-layer/data-access-objects/data-access-object-oracle.ts`:
- Around line 338-352: The IN and BETWEEN branches that handle
FilterCriteriaEnum.in and FilterCriteriaEnum.between need input validation: for
the FilterCriteriaEnum.in branch (variables inValues, field, value,
builder.whereIn) ensure you convert value to an array, trim and filter out empty
strings and if the resulting inValues.length === 0 then skip adding
builder.whereIn (or return/throw per project policy) to avoid generating invalid
SQL; for the FilterCriteriaEnum.between branch (betweenValues, field, value,
builder.whereBetween) ensure the converted array is filtered for empty entries
and has at least two items (or valid start/end) before calling
builder.whereBetween([betweenValues[0], betweenValues[1]]); apply the same
guards to the duplicate implementation in the other path so both code paths
validate and skip/handle malformed inputs instead of emitting invalid queries.
---
Outside diff comments:
In
`@shared-code/src/data-access-layer/data-access-objects/data-access-object-oracle.ts`:
- Around line 312-318: The code in data-access-object-oracle.ts is coercing
date/timestamp filter values with String(value) and converting them via
formatDate/formatTimestamp unconditionally, which corrupts array-based filters
(IN/BETWEEN); update the logic in the block that checks datesColumnsNames and
timestampColumnNames so it first detects Array.isArray(value) and either map
each element through formatDate/formatTimestamp (preserving arrays) or skip
scalar coercion so the later IN/BETWEEN handling can format elements correctly;
reference the existing symbols datesColumnsNames, timestampColumnNames,
formatDate, formatTimestamp and ensure the same array-aware change is applied to
the similar conversion code near the IN/BETWEEN handling.
---
Duplicate comments:
In
`@shared-code/src/data-access-layer/data-access-objects/data-access-object-ibmdb2.ts`:
- Around line 319-337: Guard the IN and BETWEEN branches in
data-access-object-ibmdb2.ts to avoid emitting invalid SQL or binding undefined:
in the FilterCriteriaEnum.in case (the in branch) validate inValues is a
non-empty array before creating placeholders and pushing to queryParams—if
empty, return a safe false predicate (e.g., "1=0") or skip the filter; in the
FilterCriteriaEnum.between case validate there are at least two values (or a
defined low and high) before pushing to queryParams and returning "field BETWEEN
? AND ?"; only push parameters when the corresponding values exist and handle
malformed input by returning a safe predicate or rejecting the filter so no "IN
()" or undefined binds are produced.
---
Nitpick comments:
In `@backend/test/ava-tests/non-saas-tests/non-saas-table-mysql-e2e.test.ts`:
- Around line 3669-3701: The new test case in test.serial(`${currentTest} should
return rows filtered by IN operator in body`, async (t) => { ... }) is missing
the try/catch wrapper used by other tests; wrap the entire async test body in a
try { ... } catch (e) { console.error(e); throw e; } block so errors are logged
and rethrown for consistency (locate the test by the test.serial call and the
`currentTest`-based title).
- Around line 3766-3793: The test body for test.serial(`${currentTest} should
return rows filtered by BETWEEN operator in query params`) lacks the try/catch
wrapper used elsewhere; wrap the core async test logic (everything from
obtaining connectionToTestDB through assertions) in a try { ... } catch (err) {
t.fail(err as Error) } block (or t.fail(String(err))) so failures are reported
consistently, keep the existing testTables.push(testTableName) placement (or
move it inside try before operations) and ensure the response parsing/assertions
(getTableRowsResponse, t.is, t.deepEqual) remain inside the try block so errors
are caught and handled the same way as other tests.
- Around line 3703-3730: Wrap the body of the test.serial block (the test that
checks IN operator filtering) in a try/catch similar to other tests: put the
existing logic (calls to registerUserAndReturnUserInfo, createTestTable, request
calls and assertions) inside a try, and in the catch block call t.fail(error)
(or t.fail(String(error))) to surface the failure; reference the test.serial
block and related helpers registerUserAndReturnUserInfo and createTestTable to
locate where to add the try/catch.
- Around line 3732-3764: The test block named `${currentTest} should return rows
filtered by BETWEEN operator in body` currently lacks the try/catch wrapper used
elsewhere; wrap the entire async test body (the calls to
registerUserAndReturnUserInfo, createTestTable, the POST /connection request and
the POST /table/rows/find request and assertions) in a try { ... } catch (err) {
t.fail(err); } (or t.log(err); t.fail(err);) so any runtime errors are reported
consistently — locate the test by the test.serial(...) declaration and the
helper calls registerUserAndReturnUserInfo and createTestTable to find where to
apply the wrapper.
In
`@shared-code/src/data-access-layer/data-access-objects/data-access-object-clickhouse.ts`:
- Line 3: The import of NodeClickHouseClientConfigOptions is using an internal
path; replace the current import of NodeClickHouseClientConfigOptions from
'@clickhouse/client/dist/config.js' with a package-root, type-only import
(import type { NodeClickHouseClientConfigOptions } from '@clickhouse/client') so
the code uses the public API surface; update the import statement in the module
that references NodeClickHouseClientConfigOptions (e.g., any usage in the
data-access-object-clickhouse module) to the package-root export.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 22e3af2e-cf7e-43f8-b773-33b61d836026
📒 Files selected for processing (16)
backend/src/entities/table/utils/find-filtering-fields.util.tsbackend/src/enums/filter-criteria.enum.tsbackend/test/ava-tests/non-saas-tests/non-saas-table-mysql-e2e.test.tsbackend/test/ava-tests/saas-tests/table-postgres-e2e.test.tsshared-code/src/data-access-layer/data-access-objects/data-access-object-cassandra.tsshared-code/src/data-access-layer/data-access-objects/data-access-object-clickhouse.tsshared-code/src/data-access-layer/data-access-objects/data-access-object-dynamodb.tsshared-code/src/data-access-layer/data-access-objects/data-access-object-elasticsearch.tsshared-code/src/data-access-layer/data-access-objects/data-access-object-ibmdb2.tsshared-code/src/data-access-layer/data-access-objects/data-access-object-mongodb.tsshared-code/src/data-access-layer/data-access-objects/data-access-object-mssql.tsshared-code/src/data-access-layer/data-access-objects/data-access-object-mysql.tsshared-code/src/data-access-layer/data-access-objects/data-access-object-oracle.tsshared-code/src/data-access-layer/data-access-objects/data-access-object-postgres.tsshared-code/src/data-access-layer/data-access-objects/data-access-object-redis.tsshared-code/src/shared/enums/filter-criteria.enum.ts
✅ Files skipped from review due to trivial changes (2)
- backend/src/enums/filter-criteria.enum.ts
- shared-code/src/shared/enums/filter-criteria.enum.ts
🚧 Files skipped from review as they are similar to previous changes (8)
- shared-code/src/data-access-layer/data-access-objects/data-access-object-redis.ts
- shared-code/src/data-access-layer/data-access-objects/data-access-object-cassandra.ts
- shared-code/src/data-access-layer/data-access-objects/data-access-object-elasticsearch.ts
- shared-code/src/data-access-layer/data-access-objects/data-access-object-mysql.ts
- shared-code/src/data-access-layer/data-access-objects/data-access-object-mongodb.ts
- shared-code/src/data-access-layer/data-access-objects/data-access-object-mssql.ts
- shared-code/src/data-access-layer/data-access-objects/data-access-object-postgres.ts
- shared-code/src/data-access-layer/data-access-objects/data-access-object-dynamodb.ts
Comment on lines
+91
to
+115
| if (isObjectPropertyExists(filters, `f_${fieldname}__in`)) { | ||
| const rawValue = filters[`f_${fieldname}__in`]; | ||
| filteringItems.push({ | ||
| field: fieldname, | ||
| criteria: FilterCriteriaEnum.in, | ||
| value: Array.isArray(rawValue) | ||
| ? rawValue | ||
| : String(rawValue) | ||
| .split(',') | ||
| .map((v) => v.trim()), | ||
| }); | ||
| } | ||
|
|
||
| if (isObjectPropertyExists(filters, `f_${fieldname}__between`)) { | ||
| const rawValue = filters[`f_${fieldname}__between`]; | ||
| filteringItems.push({ | ||
| field: fieldname, | ||
| criteria: FilterCriteriaEnum.between, | ||
| value: Array.isArray(rawValue) | ||
| ? rawValue | ||
| : String(rawValue) | ||
| .split(',') | ||
| .map((v) => v.trim()), | ||
| }); | ||
| } |
There was a problem hiding this comment.
Validate __in / __between values before storing filter criteria.
Line 91-115 accepts empty tokens and arbitrary between arity. That can propagate malformed filters and break DAO query generation (IN () / undefined upper bound).
Suggested fix
if (isObjectPropertyExists(filters, `f_${fieldname}__in`)) {
const rawValue = filters[`f_${fieldname}__in`];
+ const inValues = (Array.isArray(rawValue) ? rawValue : String(rawValue).split(','))
+ .map((v) => String(v).trim())
+ .filter((v) => v.length > 0);
+ if (inValues.length === 0) {
+ continue;
+ }
filteringItems.push({
field: fieldname,
criteria: FilterCriteriaEnum.in,
- value: Array.isArray(rawValue)
- ? rawValue
- : String(rawValue)
- .split(',')
- .map((v) => v.trim()),
+ value: inValues,
});
}
if (isObjectPropertyExists(filters, `f_${fieldname}__between`)) {
const rawValue = filters[`f_${fieldname}__between`];
+ const betweenValues = (Array.isArray(rawValue) ? rawValue : String(rawValue).split(','))
+ .map((v) => String(v).trim())
+ .filter((v) => v.length > 0);
+ if (betweenValues.length !== 2) {
+ throw new Error(`Invalid between filter for "${fieldname}". Expected exactly 2 values.`);
+ }
filteringItems.push({
field: fieldname,
criteria: FilterCriteriaEnum.between,
- value: Array.isArray(rawValue)
- ? rawValue
- : String(rawValue)
- .split(',')
- .map((v) => v.trim()),
+ value: betweenValues,
});
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| if (isObjectPropertyExists(filters, `f_${fieldname}__in`)) { | |
| const rawValue = filters[`f_${fieldname}__in`]; | |
| filteringItems.push({ | |
| field: fieldname, | |
| criteria: FilterCriteriaEnum.in, | |
| value: Array.isArray(rawValue) | |
| ? rawValue | |
| : String(rawValue) | |
| .split(',') | |
| .map((v) => v.trim()), | |
| }); | |
| } | |
| if (isObjectPropertyExists(filters, `f_${fieldname}__between`)) { | |
| const rawValue = filters[`f_${fieldname}__between`]; | |
| filteringItems.push({ | |
| field: fieldname, | |
| criteria: FilterCriteriaEnum.between, | |
| value: Array.isArray(rawValue) | |
| ? rawValue | |
| : String(rawValue) | |
| .split(',') | |
| .map((v) => v.trim()), | |
| }); | |
| } | |
| if (isObjectPropertyExists(filters, `f_${fieldname}__in`)) { | |
| const rawValue = filters[`f_${fieldname}__in`]; | |
| const inValues = (Array.isArray(rawValue) ? rawValue : String(rawValue).split(',')) | |
| .map((v) => String(v).trim()) | |
| .filter((v) => v.length > 0); | |
| if (inValues.length === 0) { | |
| continue; | |
| } | |
| filteringItems.push({ | |
| field: fieldname, | |
| criteria: FilterCriteriaEnum.in, | |
| value: inValues, | |
| }); | |
| } | |
| if (isObjectPropertyExists(filters, `f_${fieldname}__between`)) { | |
| const rawValue = filters[`f_${fieldname}__between`]; | |
| const betweenValues = (Array.isArray(rawValue) ? rawValue : String(rawValue).split(',')) | |
| .map((v) => String(v).trim()) | |
| .filter((v) => v.length > 0); | |
| if (betweenValues.length !== 2) { | |
| throw new Error(`Invalid between filter for "${fieldname}". Expected exactly 2 values.`); | |
| } | |
| filteringItems.push({ | |
| field: fieldname, | |
| criteria: FilterCriteriaEnum.between, | |
| value: betweenValues, | |
| }); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@backend/src/entities/table/utils/find-filtering-fields.util.ts` around lines
91 - 115, The current handling of filters[`f_${fieldname}__in`] and
filters[`f_${fieldname}__between`] can produce empty tokens or wrong arity;
before pushing into filteringItems (for FilterCriteriaEnum.in and
FilterCriteriaEnum.between) validate and normalize rawValue: convert non-array
to String(...).split(',').map(v=>v.trim()).filter(Boolean) and for __in only
push if the resulting array.length > 0; for __between ensure the resulting
array.length === 2 and both values are non-empty (or otherwise skip/throw),
using the existing symbols filters, fieldname, filteringItems,
FilterCriteriaEnum.in and FilterCriteriaEnum.between to locate and update the
logic.
Comment on lines
+128
to
129
| const filterData = filtersDataFromBody[rowName] as Record<string, unknown>; | ||
| for (const key in filterData) { |
There was a problem hiding this comment.
Guard body filter payload shape before iterating.
Line 128 assumes filtersDataFromBody[rowName] is an object. If it is null, array, or primitive, iteration can throw or produce invalid criteria parsing.
Suggested fix
- const filterData = filtersDataFromBody[rowName] as Record<string, unknown>;
+ const rawFilterData = filtersDataFromBody[rowName];
+ if (!rawFilterData || typeof rawFilterData !== 'object' || Array.isArray(rawFilterData)) {
+ throw new Error(`Invalid filters payload for "${rowName}".`);
+ }
+ const filterData = rawFilterData as Record<string, unknown>;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| const filterData = filtersDataFromBody[rowName] as Record<string, unknown>; | |
| for (const key in filterData) { | |
| const rawFilterData = filtersDataFromBody[rowName]; | |
| if (!rawFilterData || typeof rawFilterData !== 'object' || Array.isArray(rawFilterData)) { | |
| throw new Error(`Invalid filters payload for "${rowName}".`); | |
| } | |
| const filterData = rawFilterData as Record<string, unknown>; | |
| for (const key in filterData) { |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@backend/src/entities/table/utils/find-filtering-fields.util.ts` around lines
128 - 129, Guard against non-object payloads before iterating
filtersDataFromBody[rowName]: ensure the value (assigned to filterData) is a
non-null plain object (typeof === 'object' && filterData !== null &&
!Array.isArray(filterData)) and bail/continue if not, so iteration over
filterData in the loop does not throw or produce invalid criteria; apply this
check around where filtersDataFromBody, rowName, and filterData are used (in the
function in find-filtering-fields.util.ts) and handle invalid shapes by skipping
or returning an appropriate default.
Comment on lines
+264
to
+283
| case FilterCriteriaEnum.in: { | ||
| const inValues = Array.isArray(value) | ||
| ? value | ||
| : String(value) | ||
| .split(',') | ||
| .map((v) => v.trim()); | ||
| const sanitizedValues = inValues.map((v) => this.sanitizeValue(v)).join(', '); | ||
| whereClauses.push(`${escapedField} IN (${sanitizedValues})`); | ||
| break; | ||
| } | ||
| case FilterCriteriaEnum.between: { | ||
| const betweenValues = Array.isArray(value) | ||
| ? value | ||
| : String(value) | ||
| .split(',') | ||
| .map((v) => v.trim()); | ||
| whereClauses.push( | ||
| `${escapedField} BETWEEN ${this.sanitizeValue(betweenValues[0])} AND ${this.sanitizeValue(betweenValues[1])}`, | ||
| ); | ||
| break; |
There was a problem hiding this comment.
Validate IN/BETWEEN values before building ClickHouse SQL.
Line 264-283 allows empty IN lists and incomplete BETWEEN ranges, which can generate invalid SQL or unintended predicates.
Suggested fix
case FilterCriteriaEnum.in: {
- const inValues = Array.isArray(value)
+ const inValues = (Array.isArray(value)
? value
: String(value)
.split(',')
- .map((v) => v.trim());
+ .map((v) => v.trim()))
+ .filter((v) => String(v).length > 0);
+ if (inValues.length === 0) {
+ whereClauses.push('1 = 0');
+ break;
+ }
const sanitizedValues = inValues.map((v) => this.sanitizeValue(v)).join(', ');
whereClauses.push(`${escapedField} IN (${sanitizedValues})`);
break;
}
case FilterCriteriaEnum.between: {
- const betweenValues = Array.isArray(value)
+ const betweenValues = (Array.isArray(value)
? value
: String(value)
.split(',')
- .map((v) => v.trim());
+ .map((v) => v.trim()))
+ .filter((v) => String(v).length > 0);
+ if (betweenValues.length !== 2) {
+ throw new Error(`Invalid between filter for "${field}"`);
+ }
whereClauses.push(
`${escapedField} BETWEEN ${this.sanitizeValue(betweenValues[0])} AND ${this.sanitizeValue(betweenValues[1])}`,
);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@shared-code/src/data-access-layer/data-access-objects/data-access-object-clickhouse.ts`
around lines 264 - 283, The IN and BETWEEN branches in the switch (handling
FilterCriteriaEnum.in and FilterCriteriaEnum.between) currently build SQL
without validating inputs; update these branches in
data-access-object-clickhouse.ts to first normalize the incoming value into an
array, then validate: for FilterCriteriaEnum.in ensure the resulting inValues
array is non-empty (otherwise throw a clear error or skip adding the where
clause) and only then map through this.sanitizeValue and push `${escapedField}
IN (...)`; for FilterCriteriaEnum.between ensure betweenValues has at least two
non-empty elements (otherwise throw/skip) and use sanitized betweenValues[0] and
betweenValues[1] when pushing the `${escapedField} BETWEEN ... AND ...` clause;
keep using the existing this.sanitizeValue, escapedField, and whereClauses
variables to locate the code to change.
Comment on lines
+338
to
+352
| if (criteria === FilterCriteriaEnum.in) { | ||
| const inValues = Array.isArray(value) | ||
| ? value | ||
| : String(value) | ||
| .split(',') | ||
| .map((v) => v.trim()); | ||
| builder.whereIn(field, inValues); | ||
| } else if (criteria === FilterCriteriaEnum.between) { | ||
| const betweenValues = Array.isArray(value) | ||
| ? value | ||
| : String(value) | ||
| .split(',') | ||
| .map((v) => v.trim()); | ||
| builder.whereBetween(field, [betweenValues[0], betweenValues[1]]); | ||
| } else { |
There was a problem hiding this comment.
Add guards for empty IN lists and malformed BETWEEN ranges in both paths.
Line 338-352 and Line 820-835 accept empty lists and incomplete ranges without validation. This can produce invalid SQL or incorrect filtering behavior.
Suggested fix pattern (apply in both branches)
- const inValues = Array.isArray(value)
+ const inValues = (Array.isArray(value)
? value
: String(value)
.split(',')
- .map((v) => v.trim());
+ .map((v) => v.trim()))
+ .filter((v) => String(v).length > 0);
+ if (inValues.length === 0) {
+ builder.whereRaw('1 = 0');
+ continue;
+ }
builder.whereIn(field, inValues);
} else if (criteria === FilterCriteriaEnum.between) {
- const betweenValues = Array.isArray(value)
+ const betweenValues = (Array.isArray(value)
? value
: String(value)
.split(',')
- .map((v) => v.trim());
+ .map((v) => v.trim()))
+ .filter((v) => String(v).length > 0);
+ if (betweenValues.length !== 2) {
+ throw new Error(`Invalid between filter for "${field}"`);
+ }
builder.whereBetween(field, [betweenValues[0], betweenValues[1]]);Also applies to: 820-835
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@shared-code/src/data-access-layer/data-access-objects/data-access-object-oracle.ts`
around lines 338 - 352, The IN and BETWEEN branches that handle
FilterCriteriaEnum.in and FilterCriteriaEnum.between need input validation: for
the FilterCriteriaEnum.in branch (variables inValues, field, value,
builder.whereIn) ensure you convert value to an array, trim and filter out empty
strings and if the resulting inValues.length === 0 then skip adding
builder.whereIn (or return/throw per project policy) to avoid generating invalid
SQL; for the FilterCriteriaEnum.between branch (betweenValues, field, value,
builder.whereBetween) ensure the converted array is filtered for empty entries
and has at least two items (or valid start/end) before calling
builder.whereBetween([betweenValues[0], betweenValues[1]]); apply the same
guards to the duplicate implementation in the other path so both code paths
validate and skip/handle malformed inputs instead of emitting invalid queries.
The find endpoint is a query operation, not a resource creation, so it should return 200 instead of NestJS's default 201 for POST. Added @httpcode(HttpStatus.OK) and updated all stale test assertions that expected 201. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Previously the workflow ran biome on the entire repo for every push and PR, surfacing pre-existing issues in untouched files. Restrict it to pull_request events and lint only backend/ files actually changed in the PR so new violations fail fast without being drowned out by legacy noise. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/code-quality.yml:
- Around line 38-40: The "Run Biome" step is vulnerable because it interpolates
`${{ steps.changed.outputs.files }}` directly into the shell; change the step to
safely pass filenames (avoid direct shell interpolation of the variable) by
piping the output into a safe invoker like `xargs` or by iterating over the
filenames and quoting each before invocation so biome receives each path as an
argument rather than allowing shell evaluation of metacharacters; update the
step that currently runs `biome ci --formatter-enabled=false
--assist-enabled=false ${{ steps.changed.outputs.files }}` to use a safe
invocation pattern (e.g., echo/piping to xargs or a loop) to prevent command
injection.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: c42beb47-51fb-46fa-a72e-0e207ee9e311
📒 Files selected for processing (1)
.github/workflows/code-quality.yml
Comment on lines
38
to
+40
| - name: Run Biome | ||
| run: biome ci --formatter-enabled=false --assist-enabled=false . | ||
| if: steps.changed.outputs.files != '' | ||
| run: biome ci --formatter-enabled=false --assist-enabled=false ${{ steps.changed.outputs.files }} |
There was a problem hiding this comment.
Potential command injection via filename interpolation.
Directly interpolating ${{ steps.changed.outputs.files }} into the shell command is unsafe. If a PR introduces a file with shell metacharacters in its name (e.g., backend/foo$(whoami).ts), the expression is evaluated by the runner before shell execution, potentially allowing command injection.
Consider piping the file list through xargs for safer handling:
🛡️ Proposed fix using xargs
- name: Get changed backend files
id: changed
run: |
- files=$(git diff --name-only --diff-filter=ACMR \
+ git diff --name-only --diff-filter=ACMR \
"${{ github.event.pull_request.base.sha }}...${{ github.event.pull_request.head.sha }}" \
-- 'backend/' \
- | grep -E '\.(ts|tsx|js|jsx|json)$' \
- | tr '\n' ' ' || true)
- echo "files=$files" >> "$GITHUB_OUTPUT"
- echo "Changed backend files: $files"
+ | grep -E '\.(ts|tsx|js|jsx|json)$' > /tmp/changed_files.txt || true
+ if [ -s /tmp/changed_files.txt ]; then
+ echo "has_changes=true" >> "$GITHUB_OUTPUT"
+ echo "Changed backend files:"
+ cat /tmp/changed_files.txt
+ fi
- name: Run Biome
- if: steps.changed.outputs.files != ''
- run: biome ci --formatter-enabled=false --assist-enabled=false ${{ steps.changed.outputs.files }}
+ if: steps.changed.outputs.has_changes == 'true'
+ run: xargs -a /tmp/changed_files.txt biome ci --formatter-enabled=false --assist-enabled=false🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.github/workflows/code-quality.yml around lines 38 - 40, The "Run Biome"
step is vulnerable because it interpolates `${{ steps.changed.outputs.files }}`
directly into the shell; change the step to safely pass filenames (avoid direct
shell interpolation of the variable) by piping the output into a safe invoker
like `xargs` or by iterating over the filenames and quoting each before
invocation so biome receives each path as an argument rather than allowing shell
evaluation of metacharacters; update the step that currently runs `biome ci
--formatter-enabled=false --assist-enabled=false ${{ steps.changed.outputs.files
}}` to use a safe invocation pattern (e.g., echo/piping to xargs or a loop) to
prevent command injection.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
…rivers
Allow filtering rows where a column value matches any value in a provided set (e.g., status IN ('active', 'pending')). Adds the
incriteria to FilterCriteriaEnum and implements it in all 11 DAOs: Postgres, MySQL, MSSQL, Oracle (Knex whereIn), IBM DB2, Cassandra (parameterized IN), MongoDB ($in), DynamoDB (IN expression), Elasticsearch (terms query), ClickHouse (IN clause), and Redis (in-memory check).Summary by CodeRabbit
New Features
Bug Fixes / API
Tests
Chores