Skip to content

Commit 2a17d7c

Browse files
philcunliffeclaude
andcommitted
Test scanColumn cross-file LIMIT/OFFSET, mid-stream abort, and SELECT hook
Close the three review-flagged coverage gaps on scanColumn (test-only; production scanColumn was reviewed correct and is unchanged): - Cross-file LIMIT/OFFSET over the multi-data-file spark/rename_column table (3 data files) — the only fixture exercising the whole-file OFFSET skip-and-resume and the cross-file LIMIT early-break; bunnies v2/v4 both collapse to a single data file. Asserted against the scan({columns:[c]}) oracle slice, including chunk-count proof that the last file is never opened under a satisfied LIMIT. - Mid-stream abort: consume the first chunk, then abort, and assert the next pull rejects at the between-file guard (same error scan raises). - Plain single-column SELECT ... LIMIT/OFFSET routed through the scanColumn hook (execute.js fast path), asserting the hook fires and rows match the hook-disabled scan() path. - Characterization (pinned, not endorsed): N aggregates on one column currently re-scan it N times — an upstream squirreling coalescing opportunity, not an icebird bug. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent 5eb0483 commit 2a17d7c

1 file changed

Lines changed: 119 additions & 0 deletions

File tree

test/sql/icebergDataSource.test.js

Lines changed: 119 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -473,6 +473,12 @@ describe.concurrent('icebergDataSource scanColumn', () => {
473473
const tableUrl = 's3://hyperparam-iceberg/java/bunnies'
474474
const resolver = localResolver('test/files')
475475

476+
// bunnies v2/v4 each resolve to a single data file, so the cross-file
477+
// OFFSET/LIMIT and between-file abort branches need a multi-file table.
478+
// spark/rename_column has three data files walked in record-count order
479+
// (two single-row files then the two-row file; oracle id order [3,4,1,2]).
480+
const renameTableUrl = 's3://hyperparam-iceberg/spark/rename_column'
481+
476482
/**
477483
* Flatten a scanColumn stream into a single array of values, asserting each
478484
* yielded chunk is an array-like batch (the streaming/bounded-memory shape).
@@ -539,6 +545,38 @@ describe.concurrent('icebergDataSource scanColumn', () => {
539545
expect(past).toEqual([])
540546
})
541547

548+
it('honors cross-file LIMIT/OFFSET against a multi-data-file table', async () => {
549+
// The only fixture that exercises scanColumn's cross-file OFFSET whole-file
550+
// skip-and-resume and cross-file LIMIT early-break (the off-by-one-prone
551+
// sites). Asserted against the scan() oracle slice in every case.
552+
const source = await icebergDataSource({ tableUrl: renameTableUrl, resolver, metadataFileName: 'v2.metadata.json' })
553+
const full = await scanColumnOracle(source, 'id')
554+
expect(full).toHaveLength(4)
555+
556+
const { scanColumn } = source
557+
if (!scanColumn) throw new Error('scanColumn not implemented')
558+
559+
// Full-column read streams all three files, one chunk per row group/file.
560+
const { values: all, chunks: allChunks } = await drain(scanColumn({ column: 'id' }))
561+
expect(all).toEqual(full)
562+
expect(allChunks).toBe(3)
563+
564+
// (a) OFFSET that crosses a data-file boundary: offset 1 skips the whole
565+
// first file and resumes in the second; offset 2 skips the first two files;
566+
// offset 3 lands inside the final file; offset+limit may overshoot the end.
567+
for (const [offset, limit] of [[1, 2], [2, 2], [1, 10], [3, 1]]) {
568+
const { values } = await drain(scanColumn({ column: 'id', offset, limit }))
569+
expect(values).toEqual(full.slice(offset, offset + limit))
570+
}
571+
572+
// (b) LIMIT satisfied before the last file: limit 2 is filled by the first
573+
// two single-row files, so the final (two-row) file is never opened —
574+
// proven by the chunk count dropping below the full-read 3.
575+
const { values: capped, chunks: cappedChunks } = await drain(scanColumn({ column: 'id', limit: 2 }))
576+
expect(capped).toEqual(full.slice(0, 2))
577+
expect(cappedChunks).toBe(2)
578+
})
579+
542580
it('applies LIMIT/OFFSET over post-delete values when deletes are present', async () => {
543581
const source = await icebergDataSource({ tableUrl, resolver, metadataFileName: 'v4.metadata.json' })
544582
const full = await scanColumnOracle(source, 'Breed Name')
@@ -569,6 +607,27 @@ describe.concurrent('icebergDataSource scanColumn', () => {
569607
.rejects.toThrow('Aborted')
570608
})
571609

610+
it('aborts mid-stream after consuming the first chunk', async () => {
611+
// The pre-aborted case above only covers the entry guard. Use the
612+
// multi-file fixture so there is more to read after the first chunk:
613+
// consume the first file's chunk successfully, then abort, and the next
614+
// pull must reject (the same error scan raises) at the between-file guard,
615+
// honoring the JSDoc's "aborts between chunks" promise.
616+
const source = await icebergDataSource({ tableUrl: renameTableUrl, resolver, metadataFileName: 'v2.metadata.json' })
617+
const { scanColumn } = source
618+
if (!scanColumn) throw new Error('scanColumn not implemented')
619+
620+
const controller = new AbortController()
621+
const iterator = scanColumn({ column: 'id', signal: controller.signal })[Symbol.asyncIterator]()
622+
623+
const first = await iterator.next()
624+
expect(first.done).toBe(false)
625+
expect(first.value.length).toBeGreaterThanOrEqual(1)
626+
627+
controller.abort()
628+
await expect(iterator.next()).rejects.toThrow('Aborted')
629+
})
630+
572631
it('lights squirreling\'s streaming scalar-aggregate fast path', async () => {
573632
const source = await icebergDataSource({ tableUrl, resolver, metadataFileName: 'v2.metadata.json' })
574633
const baseScanColumn = source.scanColumn
@@ -601,4 +660,64 @@ describe.concurrent('icebergDataSource scanColumn', () => {
601660
expect(Number(row.s)).toBe(231)
602661
expect(Number(row.a)).toBe(11)
603662
})
663+
664+
it('serves a plain single-column SELECT with LIMIT/OFFSET through the hook', async () => {
665+
// execute.js takes the scanColumn fast path for any single-column,
666+
// WHERE-free scan, not just aggregates. Prove the hook is invoked and the
667+
// streamed rows match the ordinary scan() path (hook removed).
668+
const source = await icebergDataSource({ tableUrl, resolver, metadataFileName: 'v2.metadata.json' })
669+
const baseScanColumn = source.scanColumn
670+
if (!baseScanColumn) throw new Error('scanColumn not implemented')
671+
672+
let scanColumnCalls = 0
673+
/** @type {AsyncDataSource} */
674+
const spied = {
675+
...source,
676+
/** @type {NonNullable<AsyncDataSource['scanColumn']>} */
677+
scanColumn(options) {
678+
scanColumnCalls++
679+
return baseScanColumn(options)
680+
},
681+
}
682+
// Same source with the hook removed forces the ordinary row scan() path.
683+
/** @type {AsyncDataSource} */
684+
const noHook = { ...source, scanColumn: undefined }
685+
686+
const query = 'SELECT "Popularity Rank" FROM bunnies LIMIT 5 OFFSET 2'
687+
const viaHook = await collect(executeSql({ tables: { bunnies: spied }, query }))
688+
const viaScan = await collect(executeSql({ tables: { bunnies: noHook }, query }))
689+
690+
expect(scanColumnCalls).toBe(1)
691+
expect(viaHook).toEqual(viaScan)
692+
expect(viaHook).toHaveLength(5)
693+
})
694+
695+
it('characterizes: N aggregates on one column re-scan it N times (no coalescing)', async () => {
696+
// Current behavior, pinned for visibility — NOT an endorsement. squirreling
697+
// runs the streaming-aggregate fast path once per aggregate, so five
698+
// aggregates over one column re-scan the column five times. Coalescing them
699+
// into one pass is an upstream squirreling opportunity, not an icebird bug;
700+
// this characterization makes a future fix a visible diff here.
701+
const source = await icebergDataSource({ tableUrl, resolver, metadataFileName: 'v2.metadata.json' })
702+
const baseScanColumn = source.scanColumn
703+
if (!baseScanColumn) throw new Error('scanColumn not implemented')
704+
705+
let scanColumnCalls = 0
706+
/** @type {AsyncDataSource} */
707+
const spied = {
708+
...source,
709+
/** @type {NonNullable<AsyncDataSource['scanColumn']>} */
710+
scanColumn(options) {
711+
scanColumnCalls++
712+
return baseScanColumn(options)
713+
},
714+
}
715+
716+
await collect(executeSql({
717+
tables: { bunnies: spied },
718+
query: 'SELECT COUNT("Popularity Rank") AS c, MIN("Popularity Rank") AS mn, MAX("Popularity Rank") AS mx, SUM("Popularity Rank") AS s, AVG("Popularity Rank") AS a FROM bunnies',
719+
}))
720+
721+
expect(scanColumnCalls).toBe(5)
722+
})
604723
})

0 commit comments

Comments
 (0)