Skip to content

Commit a113e08

Browse files
fix(tables): fail fast at query batch guard
1 parent 768d71a commit a113e08

2 files changed

Lines changed: 58 additions & 9 deletions

File tree

apps/sim/lib/table/__tests__/service-filter-threading.test.ts

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -219,6 +219,19 @@ describe('queryRows byte budget', () => {
219219
updatedAt: new Date('2024-01-01'),
220220
})
221221

222+
const mockRowsThroughBatchSafetyLimit = () => {
223+
const largeRow = row(1, TABLE_LIMITS.MAX_ROW_SIZE_BYTES)
224+
const smallRow = row(2, 0)
225+
let drainBatch = 0
226+
dbChainMockFns.limit.mockResolvedValueOnce([])
227+
dbChainMockFns.limit.mockImplementation(async (ask: number) => {
228+
drainBatch++
229+
const rows = Array.from({ length: ask }, () => smallRow)
230+
if (drainBatch === 1) rows[0] = largeRow
231+
return rows
232+
})
233+
}
234+
222235
it('returns an empty page with a null cursor', async () => {
223236
const result = await queryRows(TABLE, { includeTotal: false, withExecutions: false }, 'req-1')
224237
expect(result.rows).toEqual([])
@@ -246,6 +259,26 @@ describe('queryRows byte budget', () => {
246259
expect(result.nextCursor).toBeNull()
247260
})
248261

262+
it('fails fast instead of silently truncating an unbounded query at the batch safety limit', async () => {
263+
mockRowsThroughBatchSafetyLimit()
264+
265+
await expect(
266+
queryRows(TABLE, { includeTotal: false, withExecutions: false }, 'req-1')
267+
).rejects.toThrow(/internal batches to complete safely/)
268+
})
269+
270+
it('returns a continuation cursor when a bounded query reaches the batch safety limit', async () => {
271+
mockRowsThroughBatchSafetyLimit()
272+
273+
const result = await queryRows(
274+
TABLE,
275+
{ limit: 1_000_000, includeTotal: false, withExecutions: false },
276+
'req-1'
277+
)
278+
279+
expect(result.nextCursor).not.toBeNull()
280+
})
281+
249282
it('byte-cuts a BOUNDED page and returns a resume cursor instead of throwing', async () => {
250283
const perRow = Math.floor(TABLE_LIMITS.MAX_QUERY_RESULT_BYTES * 0.6)
251284
dbChainMockFns.limit.mockResolvedValueOnce([])

apps/sim/lib/table/rows/service.ts

Lines changed: 25 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1271,13 +1271,9 @@ interface BoundedFetchResult {
12711271
}
12721272

12731273
/**
1274-
* Belt-and-braces bound on drain iterations.
1275-
*
1276-
* Unreachable only because every iteration either consumes at least one row or cuts, and a bounded
1277-
* page's `limit` is capped at {@link TABLE_LIMITS.MAX_QUERY_LIMIT} — so the limit cut always fires
1278-
* first. That makes the two constants exactly tight: raising `MAX_QUERY_LIMIT` above this bound
1279-
* would let the loop exit with rows still unread and `hasMore: false`, which clients now trust as
1280-
* end-of-table (they terminate on `nextCursor`, which this decides). Raise both together.
1274+
* Safety bound on drain iterations. Reaching it never proves source exhaustion: the drain performs
1275+
* one final witness read so bounded queries return a continuation cursor and unbounded queries fail
1276+
* fast instead of returning a partial result that looks complete.
12811277
*/
12821278
const MAX_QUERY_BATCHES = 1000
12831279

@@ -1318,6 +1314,7 @@ async function fetchRowsBounded(params: BoundedFetchParams): Promise<BoundedFetc
13181314
let anchor = params.seek
13191315
let anchorOffset = params.startOffset
13201316
let consumedSinceAnchor = 0
1317+
let sourceExhausted = false
13211318

13221319
const nextBatchRows = (): number => {
13231320
if (rows.length === 0) return Math.min(limit ?? firstBatchCap, firstBatchCap)
@@ -1369,7 +1366,10 @@ async function fetchRowsBounded(params: BoundedFetchParams): Promise<BoundedFetc
13691366
const target = Math.min(nextBatchRows(), limitRemaining)
13701367
const ask = target + 1 // +1 = witness row proving more data exists past a cut
13711368
const batch = await runBatch(anchor, anchorOffset + consumedSinceAnchor, ask)
1372-
if (batch.length === 0) break
1369+
if (batch.length === 0) {
1370+
sourceExhausted = true
1371+
break
1372+
}
13731373

13741374
let cut = false
13751375
for (const row of batch) {
@@ -1408,7 +1408,23 @@ async function fetchRowsBounded(params: BoundedFetchParams): Promise<BoundedFetc
14081408
}
14091409
if (cut) break
14101410
// Short batch = the source is exhausted; hasMore stays false.
1411-
if (batch.length < ask) break
1411+
if (batch.length < ask) {
1412+
sourceExhausted = true
1413+
break
1414+
}
1415+
}
1416+
1417+
if (!sourceExhausted && !hasMore) {
1418+
const witness = await runBatch(anchor, anchorOffset + consumedSinceAnchor, 1)
1419+
if (witness.length > 0) {
1420+
if (limit === undefined) {
1421+
throw new TableQueryValidationError(
1422+
`Query requires more than ${MAX_QUERY_BATCHES} internal batches to complete safely. Add a filter or a limit to narrow the result.`,
1423+
'TABLE_QUERY_RESULT_TOO_LARGE'
1424+
)
1425+
}
1426+
hasMore = true
1427+
}
14121428
}
14131429

14141430
return {

0 commit comments

Comments
 (0)