Skip to content

Commit 85072e9

Browse files
authored
Carry over manifest stat maps decoded by the reader (#38)
The Avro reader decodes Iceberg stat maps as arrays of {key, value} records rather than plain objects; boundForField in prune.js already handles both shapes on the read side. encodeMap only handled the object form, so writeExistingDeleteManifest threw "expected bigint value" out of avroWrite whenever it carried over an entry that had come back from a manifest with stats on it. Real delete files carry those stats: the Spark and Java position delete files under test/files/hyperparam-iceberg/*/bunnies both have lower_bounds, and the Java equality delete file has value_counts as well. Icebird alone never reached this, since v3 refuses to write new position delete files and v2 refuses to write deletion vectors, so the two only coexist on an upgraded table or one another engine wrote. The added test covers that case.
1 parent 26e85f1 commit 85072e9

2 files changed

Lines changed: 90 additions & 2 deletions

File tree

src/write/manifest.js

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -124,14 +124,18 @@ function icebergSchemaJson(schema) {
124124

125125
/**
126126
* Encode an Iceberg stat map as an Avro array of {key, value} records,
127-
* or null if the input has no entries.
127+
* or null if the input has no entries. Entries decoded by the reader already
128+
* carry that array form (see `boundForField` in prune.js), so the carry-over
129+
* writers pass one straight back through; hand-built entries use a plain
130+
* `Record<fieldId, value>`.
128131
*
129132
* @template V
130-
* @param {Record<number, V>|undefined} m
133+
* @param {Record<number, V>|{key: number, value: V}[]|undefined} m
131134
* @returns {{key: number, value: V}[]|null}
132135
*/
133136
function encodeMap(m) {
134137
if (!m) return null
138+
if (Array.isArray(m)) return m.length ? m : null
135139
const entries = Object.entries(m)
136140
if (!entries.length) return null
137141
return entries.map(([k, value]) => ({ key: Number(k), value }))

test/write/stage.deletion-vector.test.js

Lines changed: 84 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -478,6 +478,90 @@ describe('icebergStageDeletionVector', () => {
478478
expect(ids).toEqual([2n, 3n])
479479
})
480480

481+
it('carries over a position delete entry decoded by the reader', async () => {
482+
// A v2 table can accumulate position delete files across two partitions,
483+
// putting two entries in one delete manifest, each carrying the stat maps
484+
// stage-position-delete writes. Upgrading the table to v3 leaves those
485+
// files in place, so the next deletion vector delete obsoletes one entry
486+
// and has to carry the other over as EXISTING. The reader decodes Iceberg
487+
// stat maps as {key, value} record arrays rather than plain objects, so
488+
// that write has to accept the shape the reader produced.
489+
const tableUrl = 'http://test/dv-carry-over-decoded'
490+
const { resolver } = memResolver()
491+
492+
/** @type {Schema} */
493+
const partitioned = {
494+
type: 'struct',
495+
'schema-id': 0,
496+
fields: [
497+
{ id: 1, name: 'id', required: true, type: 'long' },
498+
{ id: 2, name: 'part', required: true, type: 'string' },
499+
],
500+
}
501+
const partitionSpec = {
502+
'spec-id': 0,
503+
fields: [{ 'source-id': 2, 'field-id': 1000, name: 'part', transform: 'identity' }],
504+
}
505+
const created = await icebergCreate({ tableUrl, resolver, schema: partitioned, partitionSpec })
506+
507+
// One append per partition, so each partition's data file is named by its
508+
// own staging result rather than looked up by partition value.
509+
const appendX = await icebergStageAppend({
510+
tableUrl, metadata: created, resolver,
511+
records: [{ id: 1n, part: 'x' }, { id: 2n, part: 'x' }],
512+
})
513+
const afterX = await fileCatalogCommit({ tableUrl, metadata: created, staged: appendX, resolver })
514+
const appendY = await icebergStageAppend({
515+
tableUrl, metadata: afterX, resolver,
516+
records: [{ id: 3n, part: 'y' }, { id: 4n, part: 'y' }],
517+
})
518+
const afterAppend = await fileCatalogCommit({ tableUrl, metadata: afterX, staged: appendY, resolver })
519+
const fileX = appendX.writtenFiles[0]
520+
const fileY = appendY.writtenFiles[0]
521+
522+
// One position-delete op touching both partitions: two delete files, one
523+
// manifest. A single-partition delete would leave nothing to carry over.
524+
const deletes = await icebergStagePositionDelete({
525+
tableUrl,
526+
metadata: afterAppend,
527+
deletes: [{ file_path: fileX, pos: 0n }, { file_path: fileY, pos: 0n }],
528+
resolver,
529+
})
530+
const afterDeletes = await fileCatalogCommit({ tableUrl, metadata: afterAppend, staged: deletes, resolver })
531+
const deleteManifest = deletes.writtenFiles.find(f => f.endsWith('.avro') && !f.includes('snap-'))
532+
if (!deleteManifest) throw new Error('expected a delete manifest')
533+
expect(await fetchAvroRecords(deleteManifest, resolver)).toHaveLength(2)
534+
535+
const upgraded = { ...afterDeletes, 'format-version': /** @type {3} */ (3), 'next-row-id': 0 }
536+
537+
// Targets partition x only, so its position delete entry goes obsolete
538+
// and partition y's entry is carried over.
539+
const staged = await icebergStageDeletionVector({
540+
tableUrl,
541+
metadata: upgraded,
542+
deletes: [{ file_path: fileX, pos: 1n }],
543+
resolver,
544+
})
545+
546+
const afterDv = await fileCatalogCommit({ tableUrl, metadata: upgraded, staged, resolver })
547+
const carried = (await currentManifestEntries(afterDv, resolver))
548+
.filter(e => e.data_file.content === 1 && e.status === 0)
549+
expect(carried).toHaveLength(1)
550+
// The stats came through the carry-over intact rather than being dropped
551+
// or re-encoded. The retained delete file covers position 0 of partition
552+
// y's data file, so both `pos` bounds (reserved field id 2147483545) are
553+
// the 8-byte little-endian encoding of 0.
554+
expect(carried[0].data_file.lower_bounds).toContainEqual({
555+
key: 2147483545, value: new Uint8Array(8),
556+
})
557+
expect(carried[0].data_file.upper_bounds).toContainEqual({
558+
key: 2147483545, value: new Uint8Array(8),
559+
})
560+
561+
const read = await icebergRead({ tableUrl, metadata: afterDv, resolver })
562+
expect(read.map(r => r.id).sort((a, b) => Number(a - b))).toEqual([4n])
563+
})
564+
481565
it('preserves v3 next-row-id and emits added-rows=0', async () => {
482566
vi.spyOn(Date, 'now').mockReturnValue(1700000000000)
483567
const tableUrl = 'http://test/dv-nextrow'

0 commit comments

Comments
 (0)