Add type conformance fixtures, runners, and CI by nssalian · Pull Request #4 · apache/iceberg-verification

nssalian · 2026-09-04T18:52:07Z

Rationale for this change

Iceberg implementations each parse the spec on their own and drift. Tests kept inside each implementation cannot catch a shared misreading, because each checks its own reading against itself. This adds the first conformance surface: spec-derived type fixtures (a single answer key from format/spec.md), one runner per language, and CI that reports where an implementation diverges.

Contents:

table-spec/types/ - fixtures for primitive, variant, nested, and geospatial (one cases.json per subdirectory), plus table-spec/manifest.json listing the surfaces a consumer can subscribe to. The contract is in table-spec/types/README.md: parse(input) == decoded, valid marks accept vs reject, and an optional canonical requires byte-exact re-serialization.
runners/{go,rust,python,java}/ - one runner per language; each parses input with the implementation's own type parser and reports PASS, FAIL, or UNSUPPORTED. A --surface <name> flag (or CONFORMANCE_SURFACES) runs a subset.
CI - Go, Rust, and Python build against each implementation's apache/main HEAD; Java tracks the latest published iceberg-api SNAPSHOT (nightly). Reporting is informational: a divergence is recorded as an annotation, and only a runner build or setup error marks a check failed. A nightly workflow re-runs all four and renders an aggregate matrix.
dev/validate-fixtures.py - validates each cases.json against a per-surface JSON Schema (dev/schema/), plus globally-unique ids and manifest consistency.
CONTRIBUTING.md - repository layout, the two-tier model (central runners for spec-derivable surfaces; per-implementation submodule subscription for surfaces that need implementation internals), the fixture format, and how to subscribe.

Are these changes tested?

Yes. A fork PR run exercised all four language lanes plus RAT and fixture validation: Go and Java report no divergences; pyiceberg and Rust each surface real spec divergences (pyiceberg requires a quoted geospatial CRS; Rust accepts decimal precision above 38 and an unterminated fixed[), reported as annotations. Each runner was also run locally against the implementation's apache/main, and dev/validate-fixtures.py is exercised with both valid and deliberately-broken fixtures.

Are there any user-facing changes?

No changes to any existing implementation. This adds a new, opt-in repository surface: an implementation subscribes to the fixtures it wants and runs them, and nothing here blocks another project's CI.

AI Disclosure

Model: Claude Opus 4.8
Platform/Tool: Claude Code
Human Oversight: fully reviewed; all CI run and verified by the author
Prompt Summary: build the initial type conformance surface (fixtures, per-language runners, CI), add per-surface JSON Schema validation, a surfaces manifest with a --surface runner filter, and a CONTRIBUTING guide; make CI track each implementation's dev HEAD and report divergences without blocking.

rambleraptor · 2026-09-04T19:57:26Z

+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+"""Validate the conformance fixtures.


I know this is in draft

I had pictured using JSON schema for doing the JSON validation so we didn't have to roll our own. It wouldn't handle the unique id, but it would do everything else. What are your thoughts?

Agreed. Thanks for the early feedback. Still iterating. I'll move the per-case validation to a JSON Schema and keep a thin script only for the unique-id check (and the clause/spec_ref requirement, which is types-only for this iteration then expandable).

sungwy · 2026-09-04T20:24:04Z

@@ -0,0 +1,52 @@
+{


I know this is what I had initially committed in the POC, but I began to think that maybe type string serialization is too granular to enforce.

I moved the test surface level up to the schema level here: #3

PTAL

Agreed type-string serialization is the finest grain and shouldn't gate anyone on its own. We're keeping types as its own surface, separate from the schema surface in #3 , so an impl can opt into it or skip it - the manifest plus the --surface filter in the next revision make that opt-out real, and the quoted-CRS and whitespace cases stay marked recommended, not required.

I think I'm okay with this either way, as long as the tests cover different edge cases in different surfaces, and the subscription path makes sense

sungwy

Thanks @nssalian, I think the direction is good and the POC makes the trade-offs concrete.

As I reviewed this implementation, I had a few thoughts around those trade-offs I wanted to enumerate. These are the trade-offs between having runner-based execution owned by iceberg-verification vs. a submodule subscription model owned by each implementation. Not mutually exclusive, but they optimize for different things.

Axis	Runner-based	Submodule subscription
When divergence is caught	Nightly, post-merge	Pre-merge, on the implementation's PR
Who sees the failure	This repo's CI, then relayed	The contributor who caused it
`iceberg-verification` complexity	High: one runner per language per surface	Low: fixtures plus a documented format
Implementation-side complexity	None	Medium: one submodule based integration, kept current by a dependabot
Assertion semantics authored by	This repo	The implementation that owns them

My main concern with the runners is that the runner has to reimplement assertion semantics per language and per surface. The example we have with type strings are supposed to be an easy case. Once this reaches manifests, deletion vectors, snapshot lineage or partition evolution, each surface has its assertion, and we'd be writing that multiple times, by people who don't maintain those language implementations.

I think it would be natural for a contributing on the implementation repo to want to reach for committed fixtures to test against, when they are developing within their PR. For example, if we had fixtures and references helping verify V4 manifest deletion vector handling, I can't see why contributors wouldn't want to fetch the fixtures to verify their code.

I don't think we drop runners, but these trade-offs makes me wonder if we want to use them for different levels of conformance checks. I think keeping runners would be best for nightly runs to verify a small set of high level, cross-cutting assertions where we can verify if there anything wrong with a specific implementation that should be looked into. A V4 reference-table assertion across a few reference tables is roughly the altitude I mean. Anything with low-level surface semantics feels like a better fit for the submodule path.

WDYT?

sungwy · 2026-09-04T20:30:55Z

+    if "input" not in case:
+        errors.append(f"{where}: missing 'input'")
+
+    if valid is True and "decoded" not in case:
+        errors.append(f"{where}: valid case must have 'decoded'")
+    if valid is False and "decoded" in case:
+        errors.append(f"{where}: invalid case must not have 'decoded'")


It looks like we are being prescriptive of the input and expected values format of the fixtures. Do we anticipate that it'll be the same shape in all of our cases, or is this validation script narrowly targeting the type checks?

Good point - it shouldn't be one fixed shape. In the next revision I want to move the validation to JSON Schema with one schema per surface: the types schema owns the type-string and decoded shape and requires clause/spec_ref, and the schema surface will declare its own.

I think POCing this, so that the runner adoption can scale across different surfaces would be an important pre-requisite

sungwy · 2026-09-04T20:42:10Z

+// decodedShape maps an iceberg-go type to the fixture's language-neutral
+// `decoded` shape. Numbers are float64 to match json-decoded expectations.
+// supported=false means iceberg-go has no such type.
+func decodedShape(t iceberg.Type) (map[string]any, bool) {


The need for applying language implementation specific wrapper methods to map the input and decoded values makes me want to take a step back from this approach.

This type of handling will be necessary whether that be through a iceberg-verification based runner executed model, or through a submodule based model where each implementation chooses the surfaces they want to test.

I understand why we'd want to try to keep the verification code away from the implementations, and in this repository, but I think maintaining different language implementations in a non-language repository feels like an considerable amount of overhead to me.

Yeah I agree this is a bit of an overhead. It doesn't disappear in the submodule model though. The same object-to-shape mapping still gets written, just in the impl's repo. The reason to keep it here for a surface like types is that the mapping is small and needs no impl internals, so writing it centrally means an impl gets checked without wiring anything up. For deep surfaces where the mapping needs impl internals, I agree it belongs with the impl.

sungwy · 2026-09-04T21:06:12Z

+        set +e
+        mkdir -p /tmp/status
+        out=/tmp/status/iceberg-java.txt
+        if ! ./gradlew installDist --quiet -PicebergVersion=1.12.0-SNAPSHOT ; then


good use of SNAPSHOT artifact here. Is there a way to deduce the current version, instead of pinning it so that it reduces the maintenance overhead?

I used this directly since it was already built by the nightly and I've scheduled it such that it runs after the iceberg nightly. For the others it's the latest commit hash at the time. Let me figure out how to make the version more explicit.

nssalian · 2026-09-08T17:47:56Z

Thanks for taking a look @sungwy. I put this up to help start the conversation. I think hybrid shape is right - we shouldn't stand up a runner for every surface. But I'd split by what the assertion needs, not by pre-merge vs post-merge.

Where the answer is spec-derivable with no implementation-specific knowledge, the central runner is cheap and, more to the point, needs zero buy-in from an implementation to produce a signal - an impl that never wires up a submodule still gets checked. Types is the clear case: it comes straight out of the Primitive Types table and Appendix C, so I'd keep it here as a standalone surface. Where the assertion genuinely needs impl internals (manifests, deletion vectors, snapshot lineage), I agree the impl should own the check and the submodule/pin path is the right home.

Two things I'd push on before leaning hard on the submodule model for the low-level surfaces.

First, the pre-merge visibility is real but contingent: it only exists for impls that adopt the submodule and keep the pin current, and the central runner is what covers everyone who hasn't.
Second, if an implementation owns both the code under test and the adapter that projects its object into the fixture shape, a shared misreading can conform to the fixture and hide a divergence - which is the failure this repo exists to catch. The answer key stays single-sourced either way; I just want adapter ownership to be a deliberate call. I've left similar comments in response to the others

To make the subscribe path concrete, the next revision I plan to put in adds a surfaces manifest, a --surface filter on the runners so a consumer runs only what it opts into, and a CONTRIBUTING recipe for the submodule/pin flow. Net split: runners for spec-derivable/cross-cutting surfaces, submodule for anything that needs impl internals - which keeps types central. Let me know if that boundary sounds right to you?

nssalian · 2026-09-11T20:19:10Z

The PR has a large surface area to cover. It's best to split this. I'll open smaller PRs to capture this one. I'll leave this one open until those split are ready so if anyone hasn't taken a look, can chime in here.

nssalian · 2026-09-16T15:49:49Z

I'll close this one so it's not distracting for reviews. I've begun splitting the work into the types, then the runners, etc

rambleraptor reviewed Sep 4, 2026

View reviewed changes

sungwy reviewed Sep 4, 2026

View reviewed changes

Add type conformance fixtures, runners, and CI

ca6b05e

nssalian force-pushed the types branch from ae7ff9c to ca6b05e Compare September 11, 2026 18:03

Temp per push to see if this runs

c5f6bab

nssalian closed this Sep 16, 2026

Conversation

nssalian commented Sep 4, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Rationale for this change

Are these changes tested?

Are there any user-facing changes?

AI Disclosure

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sungwy left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

nssalian commented Sep 8, 2026

Uh oh!

nssalian commented Sep 11, 2026

Uh oh!

nssalian commented Sep 16, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

nssalian commented Sep 4, 2026 •

edited

Loading