Skip to content

Fix: first append to a REST catalog table fails with a -1 ref assertion - #33

Merged
platypii merged 1 commit into
hyparam:masterfrom
ryo-rm:fix/current-snapshot-id-sentinel
Aug 12, 2026
Merged

Fix: first append to a REST catalog table fails with a -1 ref assertion#33
platypii merged 1 commit into
hyparam:masterfrom
ryo-rm:fix/current-snapshot-id-sentinel

Conversation

@ryo-rm

@ryo-rm ryo-rm commented Aug 10, 2026

Copy link
Copy Markdown
Contributor

Java writes "current-snapshot-id": -1 for "no current snapshot" on v1/v2
tables instead of omitting the field, and the spec asks other implementations
to read that sentinel as null. Icebird passed it straight through, into both
the parent-snapshot-id of the new snapshot and the assert-ref-snapshot-id
requirement sent with the commit.

The result is that no first append to a REST catalog table can succeed.
The server rejects the commit; icebird reads the 409 as a concurrent-commit
conflict and retries, so the call spends ~70s over 50 attempts before failing
with due to concurrent commits — naming the wrong cause, with the real one
visible only in the server log.

Repro

Against apache/iceberg-rest-fixture, i.e. the Java implementation itself:

docker run -d --rm --name ice-rest -p 8181:8181 apache/iceberg-rest-fixture:latest
# wait for http://localhost:8181/v1/config to answer
node repro.mjs
// repro.mjs, at the repo root
import { ByteWriter } from 'hyparquet-writer'
import { restCatalogConnect, restCatalogCreateTable } from './src/catalog/rest.js'
import { icebergAppend } from './src/write/write.js'

// In-memory storage: the catalog never reads these files during a commit.
const files = new Map()
const resolver = {
  reader(p) {
    const b = files.get(p)
    const ab = b.buffer.slice(b.byteOffset, b.byteOffset + b.byteLength)
    return { byteLength: b.byteLength, slice: (s, e) => ab.slice(s, e) }
  },
  writer(p) {
    const w = new ByteWriter()
    const finish = w.finish.bind(w)
    w.finish = async () => { await finish(); files.set(p, w.getBytes()) }
    return w
  },
}

const catalog = await restCatalogConnect({ url: 'http://localhost:8181' })
await fetch('http://localhost:8181/v1/namespaces', {
  method: 'POST',
  headers: { 'content-type': 'application/json' },
  body: JSON.stringify({ namespace: ['db'], properties: {} }),
})

const { metadata } = await restCatalogCreateTable(catalog, {
  namespace: 'db',
  table: 'orders',
  schema: {
    type: 'struct',
    'schema-id': 0,
    fields: [{ id: 1, name: 'id', required: true, type: 'long' }],
  },
  // Unrelated to this bug; the fixture defaults to zstd, which the writer
  // does not support, and that error would mask the one under test.
  properties: { 'write.parquet.compression-codec': 'snappy' },
})
console.log('current-snapshot-id from the catalog:', JSON.stringify(metadata['current-snapshot-id']))

console.time('append')
try {
  await icebergAppend({ catalog, namespace: 'db', table: 'orders', resolver, records: [{ id: 1n }] })
  console.log('append: OK')
} catch (e) {
  console.log('append: FAILED —', e.message)
}
console.timeEnd('append')

On master:

current-snapshot-id from the catalog: -1
append: FAILED — rest catalog commit failed after 50 attempts due to concurrent commits
append: 1:08.139 (m:ss.mmm)

docker logs ice-rest shows the real cause:

org.apache.iceberg.exceptions.CommitFailedException:
  Requirement failed: branch or tag main is missing, expected -1
      at org.apache.iceberg.UpdateRequirement$AssertRefSnapshotID.validate(UpdateRequirement.java:123)

which matches the REST spec — a null snapshot-id means "the ref must not
already exist", and -1 instead asserts that main points at snapshot -1.

On this branch:

current-snapshot-id from the catalog: -1
append: OK
append: 19.995ms

Fix

The file catalog path does not fail, but writes parent-snapshot-id: -1 into
metadata, pointing at a snapshot that cannot exist; the spec omits that field
for a snapshot with no parent. Every engine-written empty table in
test/files/hyperparam-iceberg carries the sentinel (java/bunnies v1,
spark/rename_column v1, athena/example 00000-*).

Normalizing only buildSnapshotUpdate would make the file catalog reject its
own commit, since checkRequirements would still compare the emitted null
against a raw -1 — so normalizeCurrentSnapshotId is applied at all three
sites that read current-snapshot-id.

Java writes `"current-snapshot-id": -1` for "no current snapshot" on v1/v2
tables instead of omitting the field, and the spec asks other implementations
to read that sentinel as null. Icebird passed it straight through, into both
the `parent-snapshot-id` of the new snapshot and the `assert-ref-snapshot-id`
requirement sent with the commit.

Reproduced against apache/iceberg-rest-fixture, which is the Java
implementation: creating a v2 table returns `"current-snapshot-id": -1`, and
the first icebergAppend is rejected server-side with

    CommitFailedException: Requirement failed: branch or tag main is missing,
    expected -1
      at UpdateRequirement$AssertRefSnapshotID.validate(UpdateRequirement.java:123)

which matches the REST spec: a null `snapshot-id` means "the ref must not
already exist", and -1 instead asserts that main points at snapshot -1. So no
first append to a REST catalog table can ever succeed. Icebird reads the 409
as a concurrent-commit conflict and retries, so the call spends ~70s over 50
attempts before failing with "due to concurrent commits" — naming the wrong
cause, with the real one visible only in the server log. After the fix the
same append commits in ~20ms.

The file catalog path does not fail, but writes `parent-snapshot-id: -1` into
metadata, pointing at a snapshot that cannot exist; the spec omits that field
for a snapshot with no parent. Every engine-written empty table in
test/files/hyperparam-iceberg carries the sentinel (java/bunnies v1,
spark/rename_column v1, athena/example 00000-*).

Normalize in one helper, `normalizeCurrentSnapshotId`, applied at all three
sites that read `current-snapshot-id`: the snapshot/requirement builder in
buildSnapshotUpdate, the local requirement check in checkRequirements, and the
ref CAS fallback in stageSetRef. Normalizing only the builder would make the
file catalog reject its own commit, since checkRequirements would still
compare the emitted null against a raw -1.
@platypii
platypii merged commit 9f049bc into hyparam:master Aug 12, 2026
3 checks passed
@platypii

Copy link
Copy Markdown
Contributor

Good catch, thanks @ryo-rm !

@ryo-rm
ryo-rm deleted the fix/current-snapshot-id-sentinel branch August 12, 2026 16:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants