You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@iceberg.apache.org by "sungwy (via GitHub)" <gi...@apache.org> on 2026/08/28 00:17:49 UTC
[PR] Add root level `CONTRIBUTING.md` [iceberg-verification]
sungwy opened a new pull request, #2:
URL: https://github.com/apache/iceberg-verification/pull/2
(no comment)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Add root level `CONTRIBUTING.md` [iceberg-verification]
Posted by "sungwy (via GitHub)" <gi...@apache.org>.
sungwy commented on code in PR #2:
URL: https://github.com/apache/iceberg-verification/pull/2#discussion_r3884394388
##########
CONTRIBUTING.md:
##########
@@ -0,0 +1,215 @@
+<!--
+ ~ Licensed to the Apache Software Foundation (ASF) under one
+ ~ or more contributor license agreements. See the NOTICE file
+ ~ distributed with this work for additional information
+ ~ regarding copyright ownership. The ASF licenses this file
+ ~ to you under the Apache License, Version 2.0 (the
+ ~ "License"); you may not use this file except in compliance
+ ~ with the License. You may obtain a copy of the License at
+ ~
+ ~ http://www.apache.org/licenses/LICENSE-2.0
+ ~
+ ~ Unless required by applicable law or agreed to in writing,
+ ~ software distributed under the License is distributed on an
+ ~ "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ ~ KIND, either express or implied. See the License for the
+ ~ specific language governing permissions and limitations
+ ~ under the License.
+ -->
+
+# Contributing
+
+First, thank you for contributing to iceberg-verification! The goal of this
+document is to provide the guidelines you need to maintain a high quality
+verification framework for all Iceberg implementations.
+
+[Iceberg](https://iceberg.apache.org/) is first and foremost a Spec, an
+agreement between different implementations on the structure and meaning of the
+artifacts that make up a table. And as such, it is crucial that the way each
+implementation interprets the spec, and the values and artifacts it stores and
+transfers as a result, stay consistent across implementations. Each
+implementation's own test suite cannot catch a divergence here, because it
+checks that implementation against its own reading of the spec.
+
+This framework aims to become a central place where the materialized value
+expectations of the spec are discussed, so that their representations and
+expectations can be used to verify implementations across languages and
+platforms.
+
+One thing to be clear about up front: we surface ambiguity here, we do not
+settle it here. When a case has no clear answer in the spec, take it to
+[dev@iceberg.apache.org](https://lists.apache.org/list.html?dev@iceberg.apache.org)
+first, and commit the fixture once the community has agreed on the answer.
+
+## Does your change belong here?
+
+This repository holds artifacts and the expected values the spec fixes for
+them. It holds no implementation code, and it verifies no behavior.
+
+If you have found a bug in one implementation, that belongs in that
+implementation's repository. Open a fixture pull request here when the spec
+fixes an expected value and an implementation disagrees with it.
+
+Engine query results, live catalog protocol behavior, and physical encoding
+choices such as compression codec and file format writer version are out of
+scope. Any spec-valid encoding is valid.
+
+## Choosing a test surface
+
+Given the above goal, it is important that contributors think deeply about what
+we are trying to verify when we introduce a new test surface.
+
+The repository consists of many test surfaces, often sub-grouped by versioned
+directories (`table-spec/v1`, `table-spec/v2`). Often, test surfaces map to
+specs. Each surface directory holds its input artifacts, the expected values
+co-located with them, and a `README.md` defining the assertion a consumer
+implements.
+
+Specs can have dependencies on other Iceberg specs. For example, Table Spec has
+a dependency on Iceberg Type Spec. In such cases, we recommend creating test
+surfaces for both layers at the root level, and focusing on the coverage that
+pertains to each level.
+
+If there are edge case value concerns with the type spec, or we want to verify
+the field structure of a type spec (e.g. Geography Type), we want to include
+that coverage in the surface for the Iceberg Type Spec. Geography Type support
+in a Table V3 spec can be added as a separate test at the Table Spec test
+surface, but the scope of testing will be different. In the Type Spec surface,
+we test edge cases in the type representation. In the Table Spec test, we check
+that V3 supports a representation of Geography Type.
+
+Notice that this is not all that different from how we reason through regular
+test suites. Keep this model in mind when you work on introducing a new test
+surface.
+
+## What should my test fixtures and expectations look like?
+
+There is no single answer here, and that is deliberate. Each combination of
+input fixture and expectation differs by the surface under test, because what
+the spec fixes differs by surface. The spec fixes the canonical form of a type
+string. It pins no such form for `metadata.json`, so two byte-different files
+can represent the same table metadata, and only the decoded values are
+comparable.
+
+It is the contributor's responsibility to understand the spec, work out the
+specification that needs to hold true, and design the input and the assertions
+that define correctness of the materialized values for that spec. Before you
+write a fixture, you should be able to answer:
+
+1. Which clause of which spec fixes the correct answer here? Link it.
+2. What is the input, and what is the smallest complete statement of the
+ expected result?
+3. What must an implementation do to check it, in one sentence? That sentence
+ belongs in the surface `README.md`, and it is the contract consumers
+ implement.
+4. Which boundary cases does the spec decide, and which does it leave open?
+
+That said, two things hold no matter which surface you are working on.
+
+### Bytes or decoded values?
+
+Where the spec fixes a canonical form, as it does for type strings, we can
+assert that re-serializing the parsed input reproduces that form exactly.
+Everywhere else, we compare the decoded logical values.
+
+It is worth thinking carefully about this, because the stricter option may not
+be the most suitable assertion. If we compare a `metadata.json` or an Avro
+manifest against an expected form byte for byte, we end up freezing one
+implementation's key ordering, and its choices about optional fields and
+codecs, as if the spec had required them. Other implementations would then fail
+our fixture for reasons the spec does not care about, and that is not what we
+want to achieve with the verification framework.
+
+When we compare decoded values, we still need to be careful about what we
+normalize away. Where the spec fixes a logical value but allows more than one
+encoding, we normalize to the value. For example, a `day` transform result is a
+`date`, and the spec requires readers to accept a plain `int` as the same date,
+so the two should compare equal
+([iceberg#16414](https://github.com/apache/iceberg/issues/16414)). Where the
+spec pins a physical type, we keep it. `equality_ids` elements are `int`, not
+`long`, so a decode that reads both as `1` would miss the defect
+([iceberg-go#880](https://github.com/apache/iceberg-go/pull/880)).
+
+So the only things we assert are the ones the spec pins. Where the spec allows
+more than one encoding of the same logical value, we normalize and there is no
+difference left to fail on. Where the spec has not decided the answer at all,
+see the next section.
+
+### Cases the spec has not settled
+
+Often the most interesting case is one where the spec does not yet give a clear
+answer, and implementations have landed in different places. For example, the
+spec caps decimal precision at 38, but whether a reader must reject
+`decimal(40, 2)` is still being worked out in
+[iceberg#16798](https://github.com/apache/iceberg/pull/16798).
+
+We do not want to leave these out, and we do not want to invent an answer for
+them either. Instead we commit the case as an open case. The input is pinned,
+no expected value is committed, and the case carries a link to where the
+question is being decided. Capturing it this way is what makes the boundary
+concrete, so that implementations can show where they currently land and the
+discussion has something specific to point at.
+
+How the open state is written is up to the surface, but every surface must make
+it possible for a consumer to tell an open case from an assertable one without
+reading the fixture by hand.
+
+An open case never fails a consumer. Running one and recording what your
+implementation does is optional, and it is the most useful thing you can bring
+back to the discussion.
+
+Once the community settles the question, the case gains its expected value and
+stops being open. Call that out in the pull request, because a consumer bumping
+its pin may start failing a case that used to pass.
+
+### Keep expected values unambiguous across implementations
+
+Your fixture will be read by Go, Python, Rust, Java, C++ consumers and other
+Iceberg implementations that may not be managed by the Apache Iceberg
+community, so the expected value itself must not be ambiguous to read. Say in
+your surface `README.md` how your expected files are written.
+
+Before inventing a convention, look at how existing surfaces handle the same
Review Comment:
I think this is a good suggestion. I've added a subsection for guidelines when using JSON as the expected value format
##########
CONTRIBUTING.md:
##########
@@ -0,0 +1,215 @@
+<!--
+ ~ Licensed to the Apache Software Foundation (ASF) under one
+ ~ or more contributor license agreements. See the NOTICE file
+ ~ distributed with this work for additional information
+ ~ regarding copyright ownership. The ASF licenses this file
+ ~ to you under the Apache License, Version 2.0 (the
+ ~ "License"); you may not use this file except in compliance
+ ~ with the License. You may obtain a copy of the License at
+ ~
+ ~ http://www.apache.org/licenses/LICENSE-2.0
+ ~
+ ~ Unless required by applicable law or agreed to in writing,
+ ~ software distributed under the License is distributed on an
+ ~ "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ ~ KIND, either express or implied. See the License for the
+ ~ specific language governing permissions and limitations
+ ~ under the License.
+ -->
+
+# Contributing
+
+First, thank you for contributing to iceberg-verification! The goal of this
+document is to provide the guidelines you need to maintain a high quality
+verification framework for all Iceberg implementations.
+
+[Iceberg](https://iceberg.apache.org/) is first and foremost a Spec, an
+agreement between different implementations on the structure and meaning of the
+artifacts that make up a table. And as such, it is crucial that the way each
+implementation interprets the spec, and the values and artifacts it stores and
+transfers as a result, stay consistent across implementations. Each
+implementation's own test suite cannot catch a divergence here, because it
+checks that implementation against its own reading of the spec.
+
+This framework aims to become a central place where the materialized value
+expectations of the spec are discussed, so that their representations and
+expectations can be used to verify implementations across languages and
+platforms.
+
+One thing to be clear about up front: we surface ambiguity here, we do not
+settle it here. When a case has no clear answer in the spec, take it to
+[dev@iceberg.apache.org](https://lists.apache.org/list.html?dev@iceberg.apache.org)
+first, and commit the fixture once the community has agreed on the answer.
+
+## Does your change belong here?
+
+This repository holds artifacts and the expected values the spec fixes for
+them. It holds no implementation code, and it verifies no behavior.
+
+If you have found a bug in one implementation, that belongs in that
+implementation's repository. Open a fixture pull request here when the spec
+fixes an expected value and an implementation disagrees with it.
+
+Engine query results, live catalog protocol behavior, and physical encoding
+choices such as compression codec and file format writer version are out of
+scope. Any spec-valid encoding is valid.
+
+## Choosing a test surface
+
+Given the above goal, it is important that contributors think deeply about what
+we are trying to verify when we introduce a new test surface.
+
+The repository consists of many test surfaces, often sub-grouped by versioned
+directories (`table-spec/v1`, `table-spec/v2`). Often, test surfaces map to
Review Comment:
Yes, that'll be nice :)
##########
CONTRIBUTING.md:
##########
@@ -0,0 +1,215 @@
+<!--
+ ~ Licensed to the Apache Software Foundation (ASF) under one
+ ~ or more contributor license agreements. See the NOTICE file
+ ~ distributed with this work for additional information
+ ~ regarding copyright ownership. The ASF licenses this file
+ ~ to you under the Apache License, Version 2.0 (the
+ ~ "License"); you may not use this file except in compliance
+ ~ with the License. You may obtain a copy of the License at
+ ~
+ ~ http://www.apache.org/licenses/LICENSE-2.0
+ ~
+ ~ Unless required by applicable law or agreed to in writing,
+ ~ software distributed under the License is distributed on an
+ ~ "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ ~ KIND, either express or implied. See the License for the
+ ~ specific language governing permissions and limitations
+ ~ under the License.
+ -->
+
+# Contributing
+
+First, thank you for contributing to iceberg-verification! The goal of this
+document is to provide the guidelines you need to maintain a high quality
Review Comment:
Yeah I thought it would be good to be forward looking, given that the community wanted to pull other forms of verification into the scope. But I agree that this is first and foremost a fixture repository and the suggested forms of verification (producer verification, reader verification) are tools that sit alongside it.
I've made some edits, feel free to take another pass
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Add root level `CONTRIBUTING.md` [iceberg-verification]
Posted by "laskoviymishka (via GitHub)" <gi...@apache.org>.
laskoviymishka commented on code in PR #2:
URL: https://github.com/apache/iceberg-verification/pull/2#discussion_r4030662487
##########
CONTRIBUTING.md:
##########
@@ -0,0 +1,277 @@
+<!--
+ ~ Licensed to the Apache Software Foundation (ASF) under one
+ ~ or more contributor license agreements. See the NOTICE file
+ ~ distributed with this work for additional information
+ ~ regarding copyright ownership. The ASF licenses this file
+ ~ to you under the Apache License, Version 2.0 (the
+ ~ "License"); you may not use this file except in compliance
+ ~ with the License. You may obtain a copy of the License at
+ ~
+ ~ http://www.apache.org/licenses/LICENSE-2.0
+ ~
+ ~ Unless required by applicable law or agreed to in writing,
+ ~ software distributed under the License is distributed on an
+ ~ "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ ~ KIND, either express or implied. See the License for the
+ ~ specific language governing permissions and limitations
+ ~ under the License.
+ -->
+
+# Contributing
+
+First, thank you for contributing to iceberg-verification! The goal of this
+document is to provide the guidelines you need to maintain high quality
+conformance fixtures for all Iceberg implementations.
+
+[Apache Iceberg](https://iceberg.apache.org/) is first and foremost a specification, an
+agreement between different implementations on the structure and meaning of the
+artifacts that make up a table. And as such, it is crucial that the way each
+implementation interprets the spec, and the values and artifacts it stores and
+transfers as a result, stay consistent across implementations. Each
+implementation's own test suite cannot catch a divergence here, because it
+checks that implementation against its own reading of the spec.
+
+This framework aims to become a central place where the materialized value
+expectations of the spec are discussed, so that their representations and
+expectations can be used to verify implementations across languages and
+platforms.
+
+One thing to be clear about up front: we surface ambiguity here, we do not
+settle it here. When a case has no clear answer in the spec, raise it on
+[dev@iceberg.apache.org](https://lists.apache.org/list.html?dev@iceberg.apache.org)
+and leave the unsettled assertion out until there is an clear expectation in the specification to write against. See
+[Cases the spec has not settled](#cases-the-spec-has-not-settled).
+
+## Does your change belong here?
+
+This repository holds artifacts and the expected values the spec fixes for
+them.
+
+If you have found a bug in one implementation, that belongs in that
+implementation's repository. Open a fixture pull request here when the spec
+fixes an expected valid or rejected value, so that existing and future
+implementations have reference values to verify against.
+
+Engine query results, live catalog protocol behavior, and physical encoding
+choices such as compression codec and file format writer version are out of
+scope. Any spec-valid encoding is valid.
+
+## Choosing a test surface
+
+Given the above goal, it is important that contributors think deeply about what
+we are trying to verify when we introduce a new test surface.
+
+A test surface is one spec behavior verified by one assertion. Each surface
+directory holds its input artifacts, the expected values co-located with them,
+and a `README.md` stating that assertion. Surfaces are grouped by the spec they
+cover, and test surfaces often map to specs. Co-located can mean an
+expected-value file beside each input, or one record carrying both. Where the
+input is small enough to write inline, one record is usually the clearer form.
+
+A surface may be divided into subdirectories along an internal dimension such
+as type or format version. The subdirectories share the surface's assertion and
+its `README.md`. A dimension that changes the assertion is not an internal
+dimension, it is a second surface.
+
+A subdirectory is the unit of subscription. It exists so that a consumer can run
+one directory to answer one conformance question, for example whether its
+implementation supports `variant` columns, and name that directory in its
+own configuration rather than filtering cases at runtime. Taking some
+subdirectories of a surface and not others is normal. Organize them so that the
+choice is expressible by path.
+
+Specs can have dependencies on other Iceberg specs. For example, Table Spec has
+a dependency on Iceberg Type Spec. In such cases, we recommend creating test
+surfaces for both layers at the root level, and focusing on the coverage that
+pertains to each level.
+
+If there are edge case value concerns with the type spec, or we want to verify
+the field structure of a type spec (e.g. Geography Type), we want to include
+that coverage in the surface for the Iceberg Type Spec. Geography Type support
+in a Table V3 spec can be added as a separate test at the Table Spec test
+surface, but the scope of testing will be different. In the Type Spec surface,
+we test edge cases in the type representation. In the Table Spec test, we check
+that V3 supports a representation of Geography Type.
+
+Notice that this is not all that different from how we reason through regular
+test suites. Keep this model in mind when you work on introducing a new test
+surface.
+
+## What should my test fixtures and expectations look like?
+
+There is no single answer here, and that is deliberate. Each combination of
+input fixture and expectation differs by the surface under test, because what
+the spec fixes differs by surface. The spec fixes the canonical form of a type
+string. It pins no such form for `metadata.json`, so two byte-different files
+can represent the same table metadata, and only the decoded values are
+comparable.
+
+It is the contributor's responsibility to understand the spec, work out the
+specification that needs to hold true, and design the input and the assertions
+that define correctness of the materialized values for that spec. Before you
+write a fixture, you should be able to answer:
+
+1. Which clause of which spec fixes the correct answer here? Link it.
+2. What is the input, and what is the smallest complete statement of the
+ expected result?
+3. What must an implementation do to check it, in one sentence? That sentence
+ belongs in the surface `README.md`, and it is the contract consumers
+ implement.
+4. Which boundary cases does the spec decide, and which does it leave open?
+
+That said, a few things hold no matter which surface you are working on.
+
+### Bytes or decoded values?
+
+Where the spec fixes a canonical form, as it does for type strings, we can
+assert that re-serializing the parsed input reproduces that form exactly.
+Everywhere else, we should compare the decoded logical values instead.
+
+It is worth thinking carefully about this, because the stricter option may not
+be the most suitable assertion. If we compare a `metadata.json` or an Avro
+manifest against an expected form byte for byte, we end up freezing one
+implementation's key ordering, and its choices about optional fields and
+codecs, as if the spec had required them. Other implementations would then fail
+our fixture for reasons the spec does not care about, and that is not what we
+want to achieve with the verification framework.
+
+When we compare decoded values, we still need to be careful about what we
+normalize away. Where the spec fixes a logical value but allows more than one
+encoding, we normalize to the value. For example, a `day` transform result is a
+`date`, and the spec requires readers to accept a plain `int` as the same date,
+so the two should compare equal. Where the spec pins a physical type, we keep
+it. `equality_ids` elements are `int`, not `long`, so a decode that reads both
+as `1` would miss the difference.
+
+So the only things we assert are the ones the spec pins. Where the spec allows
+more than one encoding of the same logical value, we normalize and there is no
+difference left to fail on. Where the spec has not decided the answer at all,
+see the next section.
+
+### Cases the spec has not settled
+
+Often the most interesting case is one where the spec does not yet give a clear
+answer, and implementations have landed in different places.
+
+Only what the spec fixes should be asserted. A surface can assert several things
+about one input, and the spec can fix some and not others. An unsettled
+serialized form does not cost a settled parse result. The assertions the spec
+fixes should be written and the rest left out.
+
+If the specification is not clear enough to guide the assertion, the question
+should be raised on
+[dev@iceberg.apache.org](https://lists.apache.org/list.html?dev@iceberg.apache.org)
+and a pull request opened holding the input. The pull request stays open while
+the question is discussed, and it is what the thread points at. It is merged
+once the community settles the answer and the expected value can be written.
+
+Where no discussion has been opened yet, the change or the clause the ambiguity
+originates in should be referenced.
+
+A surface `README.md` names the questions its inputs are waiting on, so the
+boundary is visible at a pinned commit.
+
+### Keep expected values unambiguous across implementations
+
+Your fixture will be read by Go, Python, Rust, Java, C++ consumers and other
+Iceberg implementations that may not be managed by the Apache Iceberg
+community, so the expected value itself must not be ambiguous to read. Say in
+your surface `README.md` how your expected files are written.
+
+Before inventing a convention, look at how existing surfaces handle the same
+problem and follow them where it fits. Once a convention has proven consistent
+across surfaces, we can promote it into this document as a repository-wide
+rule.
+
+#### If your expected values are JSON
+
+- **Record the type wherever the spec pins one.** JSON has a single number
+ type, so `[1]` reads the same whether an implementation decoded an `int` or
+ a `long`. `equality_ids` elements are `int`, and an expected value that
+ recorded only the number would not catch an implementation that decoded them
+ as `long`.
+- **Write 64-bit integers as strings.** Most JSON parsers read numbers as
+ doubles, which are exact only up to 2^53. A `long` value of
+ `3055729675574597004` comes back as `3055729675574597120` after such a parse,
+ with no error raised. Do this for every field the spec types as 64-bit,
+ whatever the value happens to be, so that a field is not a number in one case
+ and a string in another.
+- **Write binary as uppercase hex.** JSON has no binary type. An array of
+ integers diverges immediately, because languages disagree on whether a byte
+ is signed. A Java `byte` is signed and writes `-1` where Go and Python write
+ `255`, and base64 has more than one valid encoding of the same bytes.
+- **Write `null` out rather than leaving the field absent**, so that a reader
+ can tell a value that decoded to null from a field the assertion does not
+ cover.
+
+Override any of these where they do not fit your surface, and say why in its
+`README.md`.
+
+Some values need more than these conventions. For example, the unscaled value
+of a `decimal(38, 10)` does not fit in 64 bits at all, so a surface that covers
+one will need to say how it writes it.
+
+## What we expect of a fixture
+
+- **Derive it from the spec, do not copy it from an implementation.** The
+ implementation you copy from may already share the bug you are trying to
+ catch. Where the spec ships reference vectors, reproduce those first.
+- **Cross-check before you open the pull request.** Confirm the expected value
+ against at least one implementation other than the one that prompted the
+ fixture.
+- **Pin boundary cases, not just the spec's examples.** Drift lives at the
+ edges. Include the cases that must be rejected, not only the ones that must
+ be accepted. A corpus of valid inputs alone cannot catch a parser that is too
+ lenient.
+- **Keep committed files implementation-neutral.** Name the diverging
+ implementation in the pull request, not in the fixture.
+- **Say whether you are adding or correcting.** A consumer bumping its pin
+ needs to know whether a newly failing case is expected.
+
+## Fixture size discipline
+
+Cost is driven by commit count rather than schema width. An Avro manifest has a
+floor near 4.3 KB, since its header embeds the manifest schema, and each further
+snapshot adds a manifest list near 2 KB and a manifest of 4 to 10 KB. Measured
+against PyIceberg 0.12.0 at format version 2, a fifteen-column table with nested
+types over three snapshots comes to 25.7 KB.
+
+A few guidelines to keep fixtures from growing unnecessarily:
+
+- A fixture should be a minimal reproduction of the assertion under test.
+- Snapshot and manifest count should be kept to what that assertion requires.
+ Where an implementation is used to produce a sample table, consider committing
+ only the current `metadata.json` rather than every version the writer produced.
+- Reference tables above 100 KB should be justified in the pull request.
+
+## Opening a pull request
+
+1. Identify the surface. Reuse an existing one where the assertion already
+ fits.
+2. Add the input and its expected value to that surface.
+3. Extend the surface `README.md` if the addition changes what the assertion
+ covers.
+4. In the pull request, state the spec clause the expected value derives from,
+ which implementations you checked it against, and whether the case is an
+ addition or a correction.
+
+A new surface additionally needs a `README.md`. Section names and order are up
+to you, but it has to cover:
+
+- The assertion, stated as an expression and not only as prose.
+- What the surface covers, and what was deliberately left out and why.
+- Each subdirectory, what it covers, and whether a consumer can subscribe to it
+ on its own.
+- The shape of a case, and how to read its expected values.
+- The consumer loop, written so that two implementers cannot read it two ways.
Review Comment:
That's sound reasonable!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Add root level `CONTRIBUTING.md` [iceberg-verification]
Posted by "sungwy (via GitHub)" <gi...@apache.org>.
sungwy commented on code in PR #2:
URL: https://github.com/apache/iceberg-verification/pull/2#discussion_r3936770706
##########
CONTRIBUTING.md:
##########
@@ -0,0 +1,273 @@
+<!--
+ ~ Licensed to the Apache Software Foundation (ASF) under one
+ ~ or more contributor license agreements. See the NOTICE file
+ ~ distributed with this work for additional information
+ ~ regarding copyright ownership. The ASF licenses this file
+ ~ to you under the Apache License, Version 2.0 (the
+ ~ "License"); you may not use this file except in compliance
+ ~ with the License. You may obtain a copy of the License at
+ ~
+ ~ http://www.apache.org/licenses/LICENSE-2.0
+ ~
+ ~ Unless required by applicable law or agreed to in writing,
+ ~ software distributed under the License is distributed on an
+ ~ "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ ~ KIND, either express or implied. See the License for the
+ ~ specific language governing permissions and limitations
+ ~ under the License.
+ -->
+
+# Contributing
+
+First, thank you for contributing to iceberg-verification! The goal of this
+document is to provide the guidelines you need to maintain high quality
+conformance fixtures for all Iceberg implementations.
+
+[Iceberg](https://iceberg.apache.org/) is first and foremost a Spec, an
+agreement between different implementations on the structure and meaning of the
+artifacts that make up a table. And as such, it is crucial that the way each
+implementation interprets the spec, and the values and artifacts it stores and
+transfers as a result, stay consistent across implementations. Each
+implementation's own test suite cannot catch a divergence here, because it
+checks that implementation against its own reading of the spec.
+
+This framework aims to become a central place where the materialized value
+expectations of the spec are discussed, so that their representations and
+expectations can be used to verify implementations across languages and
+platforms.
+
+One thing to be clear about up front: we surface ambiguity here, we do not
+settle it here. When a case has no clear answer in the spec, raise it on
+[dev@iceberg.apache.org](https://lists.apache.org/list.html?dev@iceberg.apache.org),
+and commit the case in the meantime as an open case with no expected value. See
Review Comment:
Yeah I had to think through this again, rather than committing a case without expected values and having that become a convention we should just keep a PR open, continue the discussion on the PR and the mailing list and prevent the merge from happening. I'll edit this with that recomendation
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Add root level `CONTRIBUTING.md` [iceberg-verification]
Posted by "rambleraptor (via GitHub)" <gi...@apache.org>.
rambleraptor commented on code in PR #2:
URL: https://github.com/apache/iceberg-verification/pull/2#discussion_r3929343478
##########
CONTRIBUTING.md:
##########
@@ -0,0 +1,273 @@
+<!--
+ ~ Licensed to the Apache Software Foundation (ASF) under one
+ ~ or more contributor license agreements. See the NOTICE file
+ ~ distributed with this work for additional information
+ ~ regarding copyright ownership. The ASF licenses this file
+ ~ to you under the Apache License, Version 2.0 (the
+ ~ "License"); you may not use this file except in compliance
+ ~ with the License. You may obtain a copy of the License at
+ ~
+ ~ http://www.apache.org/licenses/LICENSE-2.0
+ ~
+ ~ Unless required by applicable law or agreed to in writing,
+ ~ software distributed under the License is distributed on an
+ ~ "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ ~ KIND, either express or implied. See the License for the
+ ~ specific language governing permissions and limitations
+ ~ under the License.
+ -->
+
+# Contributing
+
+First, thank you for contributing to iceberg-verification! The goal of this
+document is to provide the guidelines you need to maintain high quality
+conformance fixtures for all Iceberg implementations.
+
+[Iceberg](https://iceberg.apache.org/) is first and foremost a Spec, an
+agreement between different implementations on the structure and meaning of the
+artifacts that make up a table. And as such, it is crucial that the way each
+implementation interprets the spec, and the values and artifacts it stores and
+transfers as a result, stay consistent across implementations. Each
+implementation's own test suite cannot catch a divergence here, because it
+checks that implementation against its own reading of the spec.
+
+This framework aims to become a central place where the materialized value
+expectations of the spec are discussed, so that their representations and
+expectations can be used to verify implementations across languages and
+platforms.
+
+One thing to be clear about up front: we surface ambiguity here, we do not
+settle it here. When a case has no clear answer in the spec, raise it on
+[dev@iceberg.apache.org](https://lists.apache.org/list.html?dev@iceberg.apache.org),
+and commit the case in the meantime as an open case with no expected value. See
Review Comment:
What do you mean by "commit" the case?
The text below makes it sound like that the open case should live as a pull request and then the PR should be updated + merged when the community reaches consensus.
In my mind, commit == merging a pull request.
##########
CONTRIBUTING.md:
##########
@@ -0,0 +1,273 @@
+<!--
+ ~ Licensed to the Apache Software Foundation (ASF) under one
+ ~ or more contributor license agreements. See the NOTICE file
+ ~ distributed with this work for additional information
+ ~ regarding copyright ownership. The ASF licenses this file
+ ~ to you under the Apache License, Version 2.0 (the
+ ~ "License"); you may not use this file except in compliance
+ ~ with the License. You may obtain a copy of the License at
+ ~
+ ~ http://www.apache.org/licenses/LICENSE-2.0
+ ~
+ ~ Unless required by applicable law or agreed to in writing,
+ ~ software distributed under the License is distributed on an
+ ~ "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ ~ KIND, either express or implied. See the License for the
+ ~ specific language governing permissions and limitations
+ ~ under the License.
+ -->
+
+# Contributing
+
+First, thank you for contributing to iceberg-verification! The goal of this
+document is to provide the guidelines you need to maintain high quality
+conformance fixtures for all Iceberg implementations.
+
+[Iceberg](https://iceberg.apache.org/) is first and foremost a Spec, an
+agreement between different implementations on the structure and meaning of the
+artifacts that make up a table. And as such, it is crucial that the way each
+implementation interprets the spec, and the values and artifacts it stores and
+transfers as a result, stay consistent across implementations. Each
+implementation's own test suite cannot catch a divergence here, because it
+checks that implementation against its own reading of the spec.
Review Comment:
```suggestion
[Apache Iceberg](https://iceberg.apache.org/) is first and foremost a specification, an
agreement between different implementations on the structure and meaning of the
artifacts that make up a table. And as such, it is crucial that the way each
implementation interprets the spec, and the values and artifacts it stores and
transfers as a result, stay consistent across implementations. Each
implementation's own test suite cannot catch a divergence here, because it
checks that implementation against its own reading of the spec.
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Add root level `CONTRIBUTING.md` [iceberg-verification]
Posted by "nssalian (via GitHub)" <gi...@apache.org>.
nssalian commented on code in PR #2:
URL: https://github.com/apache/iceberg-verification/pull/2#discussion_r3882439092
##########
CONTRIBUTING.md:
##########
@@ -0,0 +1,215 @@
+<!--
+ ~ Licensed to the Apache Software Foundation (ASF) under one
+ ~ or more contributor license agreements. See the NOTICE file
+ ~ distributed with this work for additional information
+ ~ regarding copyright ownership. The ASF licenses this file
+ ~ to you under the Apache License, Version 2.0 (the
+ ~ "License"); you may not use this file except in compliance
+ ~ with the License. You may obtain a copy of the License at
+ ~
+ ~ http://www.apache.org/licenses/LICENSE-2.0
+ ~
+ ~ Unless required by applicable law or agreed to in writing,
+ ~ software distributed under the License is distributed on an
+ ~ "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ ~ KIND, either express or implied. See the License for the
+ ~ specific language governing permissions and limitations
+ ~ under the License.
+ -->
+
+# Contributing
+
+First, thank you for contributing to iceberg-verification! The goal of this
+document is to provide the guidelines you need to maintain a high quality
+verification framework for all Iceberg implementations.
+
+[Iceberg](https://iceberg.apache.org/) is first and foremost a Spec, an
+agreement between different implementations on the structure and meaning of the
+artifacts that make up a table. And as such, it is crucial that the way each
+implementation interprets the spec, and the values and artifacts it stores and
+transfers as a result, stay consistent across implementations. Each
+implementation's own test suite cannot catch a divergence here, because it
+checks that implementation against its own reading of the spec.
+
+This framework aims to become a central place where the materialized value
+expectations of the spec are discussed, so that their representations and
+expectations can be used to verify implementations across languages and
+platforms.
+
+One thing to be clear about up front: we surface ambiguity here, we do not
+settle it here. When a case has no clear answer in the spec, take it to
+[dev@iceberg.apache.org](https://lists.apache.org/list.html?dev@iceberg.apache.org)
+first, and commit the fixture once the community has agreed on the answer.
+
+## Does your change belong here?
+
+This repository holds artifacts and the expected values the spec fixes for
+them. It holds no implementation code, and it verifies no behavior.
+
+If you have found a bug in one implementation, that belongs in that
+implementation's repository. Open a fixture pull request here when the spec
+fixes an expected value and an implementation disagrees with it.
+
+Engine query results, live catalog protocol behavior, and physical encoding
+choices such as compression codec and file format writer version are out of
+scope. Any spec-valid encoding is valid.
+
+## Choosing a test surface
+
+Given the above goal, it is important that contributors think deeply about what
+we are trying to verify when we introduce a new test surface.
+
+The repository consists of many test surfaces, often sub-grouped by versioned
+directories (`table-spec/v1`, `table-spec/v2`). Often, test surfaces map to
Review Comment:
Should we decide on the directory structure before mentioning it here.
##########
CONTRIBUTING.md:
##########
@@ -0,0 +1,215 @@
+<!--
+ ~ Licensed to the Apache Software Foundation (ASF) under one
+ ~ or more contributor license agreements. See the NOTICE file
+ ~ distributed with this work for additional information
+ ~ regarding copyright ownership. The ASF licenses this file
+ ~ to you under the Apache License, Version 2.0 (the
+ ~ "License"); you may not use this file except in compliance
+ ~ with the License. You may obtain a copy of the License at
+ ~
+ ~ http://www.apache.org/licenses/LICENSE-2.0
+ ~
+ ~ Unless required by applicable law or agreed to in writing,
+ ~ software distributed under the License is distributed on an
+ ~ "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ ~ KIND, either express or implied. See the License for the
+ ~ specific language governing permissions and limitations
+ ~ under the License.
+ -->
+
+# Contributing
+
+First, thank you for contributing to iceberg-verification! The goal of this
+document is to provide the guidelines you need to maintain a high quality
+verification framework for all Iceberg implementations.
+
+[Iceberg](https://iceberg.apache.org/) is first and foremost a Spec, an
+agreement between different implementations on the structure and meaning of the
+artifacts that make up a table. And as such, it is crucial that the way each
+implementation interprets the spec, and the values and artifacts it stores and
+transfers as a result, stay consistent across implementations. Each
+implementation's own test suite cannot catch a divergence here, because it
+checks that implementation against its own reading of the spec.
+
+This framework aims to become a central place where the materialized value
+expectations of the spec are discussed, so that their representations and
+expectations can be used to verify implementations across languages and
+platforms.
+
+One thing to be clear about up front: we surface ambiguity here, we do not
+settle it here. When a case has no clear answer in the spec, take it to
+[dev@iceberg.apache.org](https://lists.apache.org/list.html?dev@iceberg.apache.org)
+first, and commit the fixture once the community has agreed on the answer.
+
+## Does your change belong here?
+
+This repository holds artifacts and the expected values the spec fixes for
+them. It holds no implementation code, and it verifies no behavior.
+
+If you have found a bug in one implementation, that belongs in that
+implementation's repository. Open a fixture pull request here when the spec
+fixes an expected value and an implementation disagrees with it.
+
+Engine query results, live catalog protocol behavior, and physical encoding
+choices such as compression codec and file format writer version are out of
+scope. Any spec-valid encoding is valid.
+
+## Choosing a test surface
+
+Given the above goal, it is important that contributors think deeply about what
+we are trying to verify when we introduce a new test surface.
+
+The repository consists of many test surfaces, often sub-grouped by versioned
+directories (`table-spec/v1`, `table-spec/v2`). Often, test surfaces map to
+specs. Each surface directory holds its input artifacts, the expected values
+co-located with them, and a `README.md` defining the assertion a consumer
+implements.
+
+Specs can have dependencies on other Iceberg specs. For example, Table Spec has
+a dependency on Iceberg Type Spec. In such cases, we recommend creating test
+surfaces for both layers at the root level, and focusing on the coverage that
+pertains to each level.
+
+If there are edge case value concerns with the type spec, or we want to verify
+the field structure of a type spec (e.g. Geography Type), we want to include
+that coverage in the surface for the Iceberg Type Spec. Geography Type support
+in a Table V3 spec can be added as a separate test at the Table Spec test
+surface, but the scope of testing will be different. In the Type Spec surface,
+we test edge cases in the type representation. In the Table Spec test, we check
+that V3 supports a representation of Geography Type.
+
+Notice that this is not all that different from how we reason through regular
+test suites. Keep this model in mind when you work on introducing a new test
+surface.
+
+## What should my test fixtures and expectations look like?
+
+There is no single answer here, and that is deliberate. Each combination of
+input fixture and expectation differs by the surface under test, because what
+the spec fixes differs by surface. The spec fixes the canonical form of a type
+string. It pins no such form for `metadata.json`, so two byte-different files
+can represent the same table metadata, and only the decoded values are
+comparable.
+
+It is the contributor's responsibility to understand the spec, work out the
+specification that needs to hold true, and design the input and the assertions
+that define correctness of the materialized values for that spec. Before you
+write a fixture, you should be able to answer:
+
+1. Which clause of which spec fixes the correct answer here? Link it.
+2. What is the input, and what is the smallest complete statement of the
+ expected result?
+3. What must an implementation do to check it, in one sentence? That sentence
+ belongs in the surface `README.md`, and it is the contract consumers
+ implement.
+4. Which boundary cases does the spec decide, and which does it leave open?
+
+That said, two things hold no matter which surface you are working on.
+
+### Bytes or decoded values?
+
+Where the spec fixes a canonical form, as it does for type strings, we can
+assert that re-serializing the parsed input reproduces that form exactly.
+Everywhere else, we compare the decoded logical values.
+
+It is worth thinking carefully about this, because the stricter option may not
+be the most suitable assertion. If we compare a `metadata.json` or an Avro
+manifest against an expected form byte for byte, we end up freezing one
+implementation's key ordering, and its choices about optional fields and
+codecs, as if the spec had required them. Other implementations would then fail
+our fixture for reasons the spec does not care about, and that is not what we
+want to achieve with the verification framework.
+
+When we compare decoded values, we still need to be careful about what we
+normalize away. Where the spec fixes a logical value but allows more than one
+encoding, we normalize to the value. For example, a `day` transform result is a
+`date`, and the spec requires readers to accept a plain `int` as the same date,
+so the two should compare equal
+([iceberg#16414](https://github.com/apache/iceberg/issues/16414)). Where the
+spec pins a physical type, we keep it. `equality_ids` elements are `int`, not
+`long`, so a decode that reads both as `1` would miss the defect
+([iceberg-go#880](https://github.com/apache/iceberg-go/pull/880)).
+
+So the only things we assert are the ones the spec pins. Where the spec allows
+more than one encoding of the same logical value, we normalize and there is no
+difference left to fail on. Where the spec has not decided the answer at all,
+see the next section.
+
+### Cases the spec has not settled
+
+Often the most interesting case is one where the spec does not yet give a clear
+answer, and implementations have landed in different places. For example, the
+spec caps decimal precision at 38, but whether a reader must reject
+`decimal(40, 2)` is still being worked out in
+[iceberg#16798](https://github.com/apache/iceberg/pull/16798).
+
+We do not want to leave these out, and we do not want to invent an answer for
+them either. Instead we commit the case as an open case. The input is pinned,
+no expected value is committed, and the case carries a link to where the
+question is being decided. Capturing it this way is what makes the boundary
+concrete, so that implementations can show where they currently land and the
+discussion has something specific to point at.
+
+How the open state is written is up to the surface, but every surface must make
+it possible for a consumer to tell an open case from an assertable one without
+reading the fixture by hand.
+
+An open case never fails a consumer. Running one and recording what your
+implementation does is optional, and it is the most useful thing you can bring
+back to the discussion.
+
+Once the community settles the question, the case gains its expected value and
+stops being open. Call that out in the pull request, because a consumer bumping
+its pin may start failing a case that used to pass.
+
+### Keep expected values unambiguous across implementations
+
+Your fixture will be read by Go, Python, Rust, Java, C++ consumers and other
+Iceberg implementations that may not be managed by the Apache Iceberg
+community, so the expected value itself must not be ambiguous to read. Say in
+your surface `README.md` how your expected files are written.
+
+Before inventing a convention, look at how existing surfaces handle the same
Review Comment:
The design doc pins concrete, language-neutral encoding rules for expected values (typed fields, 64-bit ints as strings,
binary as uppercase hex, `null` explicit). Here we defer all of this to per-surface READMEs, so early surfaces may each reinvent and diverge. Maybe we should state them as repo-wide defaults with a per-surface override. Something like this:
```
Unless a surface's `README.md` states otherwise, expected values use explicitly typed fields, 64-bit integers as strings, binary as uppercase hex, and explicit `null`. Override only where the spec forces it, and say so in the `README.md`.
```
##########
CONTRIBUTING.md:
##########
@@ -0,0 +1,215 @@
+<!--
+ ~ Licensed to the Apache Software Foundation (ASF) under one
+ ~ or more contributor license agreements. See the NOTICE file
+ ~ distributed with this work for additional information
+ ~ regarding copyright ownership. The ASF licenses this file
+ ~ to you under the Apache License, Version 2.0 (the
+ ~ "License"); you may not use this file except in compliance
+ ~ with the License. You may obtain a copy of the License at
+ ~
+ ~ http://www.apache.org/licenses/LICENSE-2.0
+ ~
+ ~ Unless required by applicable law or agreed to in writing,
+ ~ software distributed under the License is distributed on an
+ ~ "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ ~ KIND, either express or implied. See the License for the
+ ~ specific language governing permissions and limitations
+ ~ under the License.
+ -->
+
+# Contributing
+
+First, thank you for contributing to iceberg-verification! The goal of this
+document is to provide the guidelines you need to maintain a high quality
Review Comment:
This might be worth clarifying initially as a fixtures repository versus a full verification framework so users have the right expectation. We can modify this as it expands to a full framework for sure.
##########
CONTRIBUTING.md:
##########
@@ -0,0 +1,215 @@
+<!--
+ ~ Licensed to the Apache Software Foundation (ASF) under one
+ ~ or more contributor license agreements. See the NOTICE file
+ ~ distributed with this work for additional information
+ ~ regarding copyright ownership. The ASF licenses this file
+ ~ to you under the Apache License, Version 2.0 (the
+ ~ "License"); you may not use this file except in compliance
+ ~ with the License. You may obtain a copy of the License at
+ ~
+ ~ http://www.apache.org/licenses/LICENSE-2.0
+ ~
+ ~ Unless required by applicable law or agreed to in writing,
+ ~ software distributed under the License is distributed on an
+ ~ "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ ~ KIND, either express or implied. See the License for the
+ ~ specific language governing permissions and limitations
+ ~ under the License.
+ -->
+
+# Contributing
+
+First, thank you for contributing to iceberg-verification! The goal of this
+document is to provide the guidelines you need to maintain a high quality
+verification framework for all Iceberg implementations.
+
+[Iceberg](https://iceberg.apache.org/) is first and foremost a Spec, an
+agreement between different implementations on the structure and meaning of the
+artifacts that make up a table. And as such, it is crucial that the way each
+implementation interprets the spec, and the values and artifacts it stores and
+transfers as a result, stay consistent across implementations. Each
+implementation's own test suite cannot catch a divergence here, because it
+checks that implementation against its own reading of the spec.
+
+This framework aims to become a central place where the materialized value
+expectations of the spec are discussed, so that their representations and
+expectations can be used to verify implementations across languages and
+platforms.
+
+One thing to be clear about up front: we surface ambiguity here, we do not
+settle it here. When a case has no clear answer in the spec, take it to
+[dev@iceberg.apache.org](https://lists.apache.org/list.html?dev@iceberg.apache.org)
+first, and commit the fixture once the community has agreed on the answer.
+
+## Does your change belong here?
+
+This repository holds artifacts and the expected values the spec fixes for
+them. It holds no implementation code, and it verifies no behavior.
+
+If you have found a bug in one implementation, that belongs in that
+implementation's repository. Open a fixture pull request here when the spec
+fixes an expected value and an implementation disagrees with it.
+
+Engine query results, live catalog protocol behavior, and physical encoding
+choices such as compression codec and file format writer version are out of
+scope. Any spec-valid encoding is valid.
+
+## Choosing a test surface
+
+Given the above goal, it is important that contributors think deeply about what
+we are trying to verify when we introduce a new test surface.
+
+The repository consists of many test surfaces, often sub-grouped by versioned
+directories (`table-spec/v1`, `table-spec/v2`). Often, test surfaces map to
+specs. Each surface directory holds its input artifacts, the expected values
+co-located with them, and a `README.md` defining the assertion a consumer
+implements.
+
+Specs can have dependencies on other Iceberg specs. For example, Table Spec has
+a dependency on Iceberg Type Spec. In such cases, we recommend creating test
+surfaces for both layers at the root level, and focusing on the coverage that
+pertains to each level.
+
+If there are edge case value concerns with the type spec, or we want to verify
+the field structure of a type spec (e.g. Geography Type), we want to include
+that coverage in the surface for the Iceberg Type Spec. Geography Type support
+in a Table V3 spec can be added as a separate test at the Table Spec test
+surface, but the scope of testing will be different. In the Type Spec surface,
+we test edge cases in the type representation. In the Table Spec test, we check
+that V3 supports a representation of Geography Type.
+
+Notice that this is not all that different from how we reason through regular
+test suites. Keep this model in mind when you work on introducing a new test
+surface.
+
+## What should my test fixtures and expectations look like?
+
+There is no single answer here, and that is deliberate. Each combination of
+input fixture and expectation differs by the surface under test, because what
+the spec fixes differs by surface. The spec fixes the canonical form of a type
+string. It pins no such form for `metadata.json`, so two byte-different files
+can represent the same table metadata, and only the decoded values are
+comparable.
+
+It is the contributor's responsibility to understand the spec, work out the
+specification that needs to hold true, and design the input and the assertions
+that define correctness of the materialized values for that spec. Before you
+write a fixture, you should be able to answer:
+
+1. Which clause of which spec fixes the correct answer here? Link it.
+2. What is the input, and what is the smallest complete statement of the
+ expected result?
+3. What must an implementation do to check it, in one sentence? That sentence
+ belongs in the surface `README.md`, and it is the contract consumers
+ implement.
+4. Which boundary cases does the spec decide, and which does it leave open?
+
+That said, two things hold no matter which surface you are working on.
+
+### Bytes or decoded values?
+
+Where the spec fixes a canonical form, as it does for type strings, we can
+assert that re-serializing the parsed input reproduces that form exactly.
+Everywhere else, we compare the decoded logical values.
+
+It is worth thinking carefully about this, because the stricter option may not
+be the most suitable assertion. If we compare a `metadata.json` or an Avro
+manifest against an expected form byte for byte, we end up freezing one
+implementation's key ordering, and its choices about optional fields and
+codecs, as if the spec had required them. Other implementations would then fail
+our fixture for reasons the spec does not care about, and that is not what we
+want to achieve with the verification framework.
+
+When we compare decoded values, we still need to be careful about what we
+normalize away. Where the spec fixes a logical value but allows more than one
+encoding, we normalize to the value. For example, a `day` transform result is a
+`date`, and the spec requires readers to accept a plain `int` as the same date,
+so the two should compare equal
+([iceberg#16414](https://github.com/apache/iceberg/issues/16414)). Where the
+spec pins a physical type, we keep it. `equality_ids` elements are `int`, not
+`long`, so a decode that reads both as `1` would miss the defect
+([iceberg-go#880](https://github.com/apache/iceberg-go/pull/880)).
+
+So the only things we assert are the ones the spec pins. Where the spec allows
+more than one encoding of the same logical value, we normalize and there is no
+difference left to fail on. Where the spec has not decided the answer at all,
+see the next section.
+
+### Cases the spec has not settled
+
+Often the most interesting case is one where the spec does not yet give a clear
+answer, and implementations have landed in different places. For example, the
+spec caps decimal precision at 38, but whether a reader must reject
+`decimal(40, 2)` is still being worked out in
+[iceberg#16798](https://github.com/apache/iceberg/pull/16798).
+
+We do not want to leave these out, and we do not want to invent an answer for
+them either. Instead we commit the case as an open case. The input is pinned,
+no expected value is committed, and the case carries a link to where the
+question is being decided. Capturing it this way is what makes the boundary
+concrete, so that implementations can show where they currently land and the
+discussion has something specific to point at.
+
+How the open state is written is up to the surface, but every surface must make
+it possible for a consumer to tell an open case from an assertable one without
+reading the fixture by hand.
+
+An open case never fails a consumer. Running one and recording what your
+implementation does is optional, and it is the most useful thing you can bring
+back to the discussion.
+
+Once the community settles the question, the case gains its expected value and
+stops being open. Call that out in the pull request, because a consumer bumping
+its pin may start failing a case that used to pass.
+
+### Keep expected values unambiguous across implementations
+
+Your fixture will be read by Go, Python, Rust, Java, C++ consumers and other
+Iceberg implementations that may not be managed by the Apache Iceberg
+community, so the expected value itself must not be ambiguous to read. Say in
+your surface `README.md` how your expected files are written.
+
+Before inventing a convention, look at how existing surfaces handle the same
+problem and follow them where it fits. Once a convention has proven consistent
+across surfaces, we can promote it into this document as a repository-wide
+rule.
+
+## What we expect of a fixture
+
+- **Derive it from the spec, do not copy it from an implementation.** The
Review Comment:
The design doc lists "one generator of record for binary inputs (Java, the reference)" as a contributor expectation. Here we cover derive-from-spec and cross-check, but never says how to produce a new Parquet/Avro/manifest fixture.
Worth adding another bullet under this section to say:
```
Generate binary inputs from the reference generator. Produce Parquet/Avro/manifest inputs from Java, derive the expected value from the spec separately, and cross-check against a second implementation before opening the PR.
```
Something like this? https://github.com/apache/parquet-testing/pull/118
##########
CONTRIBUTING.md:
##########
@@ -0,0 +1,215 @@
+<!--
+ ~ Licensed to the Apache Software Foundation (ASF) under one
+ ~ or more contributor license agreements. See the NOTICE file
+ ~ distributed with this work for additional information
+ ~ regarding copyright ownership. The ASF licenses this file
+ ~ to you under the Apache License, Version 2.0 (the
+ ~ "License"); you may not use this file except in compliance
+ ~ with the License. You may obtain a copy of the License at
+ ~
+ ~ http://www.apache.org/licenses/LICENSE-2.0
+ ~
+ ~ Unless required by applicable law or agreed to in writing,
+ ~ software distributed under the License is distributed on an
+ ~ "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ ~ KIND, either express or implied. See the License for the
+ ~ specific language governing permissions and limitations
+ ~ under the License.
+ -->
+
+# Contributing
+
+First, thank you for contributing to iceberg-verification! The goal of this
+document is to provide the guidelines you need to maintain a high quality
+verification framework for all Iceberg implementations.
+
+[Iceberg](https://iceberg.apache.org/) is first and foremost a Spec, an
+agreement between different implementations on the structure and meaning of the
+artifacts that make up a table. And as such, it is crucial that the way each
+implementation interprets the spec, and the values and artifacts it stores and
+transfers as a result, stay consistent across implementations. Each
+implementation's own test suite cannot catch a divergence here, because it
+checks that implementation against its own reading of the spec.
+
+This framework aims to become a central place where the materialized value
+expectations of the spec are discussed, so that their representations and
+expectations can be used to verify implementations across languages and
+platforms.
+
+One thing to be clear about up front: we surface ambiguity here, we do not
+settle it here. When a case has no clear answer in the spec, take it to
+[dev@iceberg.apache.org](https://lists.apache.org/list.html?dev@iceberg.apache.org)
+first, and commit the fixture once the community has agreed on the answer.
+
+## Does your change belong here?
+
+This repository holds artifacts and the expected values the spec fixes for
+them. It holds no implementation code, and it verifies no behavior.
+
+If you have found a bug in one implementation, that belongs in that
+implementation's repository. Open a fixture pull request here when the spec
+fixes an expected value and an implementation disagrees with it.
+
+Engine query results, live catalog protocol behavior, and physical encoding
+choices such as compression codec and file format writer version are out of
+scope. Any spec-valid encoding is valid.
+
+## Choosing a test surface
+
+Given the above goal, it is important that contributors think deeply about what
+we are trying to verify when we introduce a new test surface.
+
+The repository consists of many test surfaces, often sub-grouped by versioned
+directories (`table-spec/v1`, `table-spec/v2`). Often, test surfaces map to
+specs. Each surface directory holds its input artifacts, the expected values
+co-located with them, and a `README.md` defining the assertion a consumer
+implements.
+
+Specs can have dependencies on other Iceberg specs. For example, Table Spec has
+a dependency on Iceberg Type Spec. In such cases, we recommend creating test
+surfaces for both layers at the root level, and focusing on the coverage that
+pertains to each level.
+
+If there are edge case value concerns with the type spec, or we want to verify
+the field structure of a type spec (e.g. Geography Type), we want to include
+that coverage in the surface for the Iceberg Type Spec. Geography Type support
+in a Table V3 spec can be added as a separate test at the Table Spec test
+surface, but the scope of testing will be different. In the Type Spec surface,
+we test edge cases in the type representation. In the Table Spec test, we check
+that V3 supports a representation of Geography Type.
+
+Notice that this is not all that different from how we reason through regular
+test suites. Keep this model in mind when you work on introducing a new test
+surface.
+
+## What should my test fixtures and expectations look like?
+
+There is no single answer here, and that is deliberate. Each combination of
+input fixture and expectation differs by the surface under test, because what
+the spec fixes differs by surface. The spec fixes the canonical form of a type
+string. It pins no such form for `metadata.json`, so two byte-different files
+can represent the same table metadata, and only the decoded values are
+comparable.
+
+It is the contributor's responsibility to understand the spec, work out the
+specification that needs to hold true, and design the input and the assertions
+that define correctness of the materialized values for that spec. Before you
+write a fixture, you should be able to answer:
+
+1. Which clause of which spec fixes the correct answer here? Link it.
+2. What is the input, and what is the smallest complete statement of the
+ expected result?
+3. What must an implementation do to check it, in one sentence? That sentence
+ belongs in the surface `README.md`, and it is the contract consumers
+ implement.
+4. Which boundary cases does the spec decide, and which does it leave open?
+
+That said, two things hold no matter which surface you are working on.
+
+### Bytes or decoded values?
+
+Where the spec fixes a canonical form, as it does for type strings, we can
+assert that re-serializing the parsed input reproduces that form exactly.
+Everywhere else, we compare the decoded logical values.
+
+It is worth thinking carefully about this, because the stricter option may not
+be the most suitable assertion. If we compare a `metadata.json` or an Avro
+manifest against an expected form byte for byte, we end up freezing one
+implementation's key ordering, and its choices about optional fields and
+codecs, as if the spec had required them. Other implementations would then fail
+our fixture for reasons the spec does not care about, and that is not what we
+want to achieve with the verification framework.
+
+When we compare decoded values, we still need to be careful about what we
+normalize away. Where the spec fixes a logical value but allows more than one
+encoding, we normalize to the value. For example, a `day` transform result is a
+`date`, and the spec requires readers to accept a plain `int` as the same date,
+so the two should compare equal
+([iceberg#16414](https://github.com/apache/iceberg/issues/16414)). Where the
+spec pins a physical type, we keep it. `equality_ids` elements are `int`, not
+`long`, so a decode that reads both as `1` would miss the defect
+([iceberg-go#880](https://github.com/apache/iceberg-go/pull/880)).
+
+So the only things we assert are the ones the spec pins. Where the spec allows
+more than one encoding of the same logical value, we normalize and there is no
+difference left to fail on. Where the spec has not decided the answer at all,
+see the next section.
+
+### Cases the spec has not settled
+
+Often the most interesting case is one where the spec does not yet give a clear
+answer, and implementations have landed in different places. For example, the
+spec caps decimal precision at 38, but whether a reader must reject
+`decimal(40, 2)` is still being worked out in
+[iceberg#16798](https://github.com/apache/iceberg/pull/16798).
+
+We do not want to leave these out, and we do not want to invent an answer for
+them either. Instead we commit the case as an open case. The input is pinned,
+no expected value is committed, and the case carries a link to where the
+question is being decided. Capturing it this way is what makes the boundary
+concrete, so that implementations can show where they currently land and the
+discussion has something specific to point at.
+
+How the open state is written is up to the surface, but every surface must make
+it possible for a consumer to tell an open case from an assertable one without
+reading the fixture by hand.
+
+An open case never fails a consumer. Running one and recording what your
+implementation does is optional, and it is the most useful thing you can bring
+back to the discussion.
+
+Once the community settles the question, the case gains its expected value and
+stops being open. Call that out in the pull request, because a consumer bumping
+its pin may start failing a case that used to pass.
+
+### Keep expected values unambiguous across implementations
+
+Your fixture will be read by Go, Python, Rust, Java, C++ consumers and other
+Iceberg implementations that may not be managed by the Apache Iceberg
+community, so the expected value itself must not be ambiguous to read. Say in
+your surface `README.md` how your expected files are written.
+
+Before inventing a convention, look at how existing surfaces handle the same
+problem and follow them where it fits. Once a convention has proven consistent
+across surfaces, we can promote it into this document as a repository-wide
+rule.
+
+## What we expect of a fixture
+
+- **Derive it from the spec, do not copy it from an implementation.** The
+ implementation you copy from may already share the bug you are trying to
+ catch. Where the spec ships reference vectors, reproduce those first.
+- **Cross-check before you open the pull request.** Confirm the expected value
+ against at least one implementation other than the one that prompted the
+ fixture.
+- **Pin boundary cases, not just the spec's examples.** Drift lives at the
+ edges. Include the cases that must be rejected, not only the ones that must
+ be accepted. A corpus of valid inputs alone cannot catch a parser that is too
+ lenient.
+- **Keep committed files implementation-neutral.** Name the diverging
+ implementation in the pull request, not in the fixture.
+- **Say whether you are adding or correcting.** A consumer bumping its pin
+ needs to know whether a newly failing case is expected.
+
+## Opening a pull request
+
+1. Identify the surface. Reuse an existing one where the assertion already
+ fits.
+2. Add the input and its expected value to that surface.
+3. Extend the surface `README.md` if the addition changes what the assertion
+ covers.
+4. In the pull request, state the spec clause the expected value derives from,
+ which implementations you checked it against, and whether the case is an
+ addition or a correction.
+
+A new surface additionally needs a `README.md` defining its assertion, the
+shape of its inputs and expected values.
+
+## License headers
+
+Please do not add license headers to fixture files. They are test data, and a
+header would alter the artifact under test. The
+[ASF Source Header and Copyright Notice Policy](https://www.apache.org/legal/src-headers.html)
+exempts "test data for which the addition of a source header would cause the
Review Comment:
The license checks should exclude the right directories eventually
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Add root level `CONTRIBUTING.md` [iceberg-verification]
Posted by "sungwy (via GitHub)" <gi...@apache.org>.
sungwy commented on code in PR #2:
URL: https://github.com/apache/iceberg-verification/pull/2#discussion_r4028835716
##########
CONTRIBUTING.md:
##########
@@ -0,0 +1,277 @@
+<!--
+ ~ Licensed to the Apache Software Foundation (ASF) under one
+ ~ or more contributor license agreements. See the NOTICE file
+ ~ distributed with this work for additional information
+ ~ regarding copyright ownership. The ASF licenses this file
+ ~ to you under the Apache License, Version 2.0 (the
+ ~ "License"); you may not use this file except in compliance
+ ~ with the License. You may obtain a copy of the License at
+ ~
+ ~ http://www.apache.org/licenses/LICENSE-2.0
+ ~
+ ~ Unless required by applicable law or agreed to in writing,
+ ~ software distributed under the License is distributed on an
+ ~ "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ ~ KIND, either express or implied. See the License for the
+ ~ specific language governing permissions and limitations
+ ~ under the License.
+ -->
+
+# Contributing
+
+First, thank you for contributing to iceberg-verification! The goal of this
+document is to provide the guidelines you need to maintain high quality
+conformance fixtures for all Iceberg implementations.
+
+[Apache Iceberg](https://iceberg.apache.org/) is first and foremost a specification, an
+agreement between different implementations on the structure and meaning of the
+artifacts that make up a table. And as such, it is crucial that the way each
+implementation interprets the spec, and the values and artifacts it stores and
+transfers as a result, stay consistent across implementations. Each
+implementation's own test suite cannot catch a divergence here, because it
+checks that implementation against its own reading of the spec.
+
+This framework aims to become a central place where the materialized value
+expectations of the spec are discussed, so that their representations and
+expectations can be used to verify implementations across languages and
+platforms.
+
+One thing to be clear about up front: we surface ambiguity here, we do not
+settle it here. When a case has no clear answer in the spec, raise it on
+[dev@iceberg.apache.org](https://lists.apache.org/list.html?dev@iceberg.apache.org)
+and leave the unsettled assertion out until there is an clear expectation in the specification to write against. See
+[Cases the spec has not settled](#cases-the-spec-has-not-settled).
+
+## Does your change belong here?
+
+This repository holds artifacts and the expected values the spec fixes for
+them.
+
+If you have found a bug in one implementation, that belongs in that
+implementation's repository. Open a fixture pull request here when the spec
+fixes an expected valid or rejected value, so that existing and future
+implementations have reference values to verify against.
+
+Engine query results, live catalog protocol behavior, and physical encoding
+choices such as compression codec and file format writer version are out of
+scope. Any spec-valid encoding is valid.
+
+## Choosing a test surface
+
+Given the above goal, it is important that contributors think deeply about what
+we are trying to verify when we introduce a new test surface.
+
+A test surface is one spec behavior verified by one assertion. Each surface
Review Comment:
I like that wording @laskoviymishka - thanks!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Add root level `CONTRIBUTING.md` [iceberg-verification]
Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #2:
URL: https://github.com/apache/iceberg-verification/pull/2#discussion_r3876857339
##########
CONTRIBUTING.md:
##########
@@ -0,0 +1,215 @@
+<!--
+ ~ Licensed to the Apache Software Foundation (ASF) under one
+ ~ or more contributor license agreements. See the NOTICE file
+ ~ distributed with this work for additional information
+ ~ regarding copyright ownership. The ASF licenses this file
+ ~ to you under the Apache License, Version 2.0 (the
+ ~ "License"); you may not use this file except in compliance
+ ~ with the License. You may obtain a copy of the License at
+ ~
+ ~ http://www.apache.org/licenses/LICENSE-2.0
+ ~
+ ~ Unless required by applicable law or agreed to in writing,
+ ~ software distributed under the License is distributed on an
+ ~ "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ ~ KIND, either express or implied. See the License for the
+ ~ specific language governing permissions and limitations
+ ~ under the License.
+ -->
+
+# Contributing
+
+First, thank you for contributing to iceberg-verification! The goal of this
+document is to provide the guidelines you need to maintain a high quality
+verification framework for all Iceberg implementations.
+
+[Iceberg](https://iceberg.apache.org/) is first and foremost a Spec, an
+agreement between different implementations on the structure and meaning of the
+artifacts that make up a table. And as such, it is crucial that the way each
+implementation interprets the spec, and the values and artifacts it stores and
+transfers as a result, stay consistent across implementations. Each
+implementation's own test suite cannot catch a divergence here, because it
+checks that implementation against its own reading of the spec.
+
+This framework aims to become a central place where the materialized value
+expectations of the spec are discussed, so that their representations and
+expectations can be used to verify implementations across languages and
+platforms.
+
+One thing to be clear about up front: we surface ambiguity here, we do not
+settle it here. When a case has no clear answer in the spec, take it to
+[dev@iceberg.apache.org](https://lists.apache.org/list.html?dev@iceberg.apache.org)
+first, and commit the fixture once the community has agreed on the answer.
+
+## Does your change belong here?
+
+This repository holds artifacts and the expected values the spec fixes for
+them. It holds no implementation code, and it verifies no behavior.
Review Comment:
The statement that the repo "holds no implementation code" and "verifies no behavior" conflicts with the repository README, which describes this as a verification framework with runners that exercise fixtures. Consider rewording to avoid an absolute claim and align with the README’s positioning.
##########
.asf.yaml:
##########
@@ -53,6 +53,9 @@ github:
issues: false
wiki: false
projects: false
+
+ collaborators: # Note: the number of collaborators is limited to 10
Review Comment:
There is trailing whitespace on the blank line before `collaborators:` (line 56). This can cause unnecessary diffs and may trip whitespace/lint checks; prefer an empty line with no spaces.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Add root level `CONTRIBUTING.md` [iceberg-verification]
Posted by "laskoviymishka (via GitHub)" <gi...@apache.org>.
laskoviymishka commented on code in PR #2:
URL: https://github.com/apache/iceberg-verification/pull/2#discussion_r4024690621
##########
CONTRIBUTING.md:
##########
@@ -0,0 +1,277 @@
+<!--
+ ~ Licensed to the Apache Software Foundation (ASF) under one
+ ~ or more contributor license agreements. See the NOTICE file
+ ~ distributed with this work for additional information
+ ~ regarding copyright ownership. The ASF licenses this file
+ ~ to you under the Apache License, Version 2.0 (the
+ ~ "License"); you may not use this file except in compliance
+ ~ with the License. You may obtain a copy of the License at
+ ~
+ ~ http://www.apache.org/licenses/LICENSE-2.0
+ ~
+ ~ Unless required by applicable law or agreed to in writing,
+ ~ software distributed under the License is distributed on an
+ ~ "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ ~ KIND, either express or implied. See the License for the
+ ~ specific language governing permissions and limitations
+ ~ under the License.
+ -->
+
+# Contributing
+
+First, thank you for contributing to iceberg-verification! The goal of this
+document is to provide the guidelines you need to maintain high quality
+conformance fixtures for all Iceberg implementations.
+
+[Apache Iceberg](https://iceberg.apache.org/) is first and foremost a specification, an
+agreement between different implementations on the structure and meaning of the
+artifacts that make up a table. And as such, it is crucial that the way each
+implementation interprets the spec, and the values and artifacts it stores and
+transfers as a result, stay consistent across implementations. Each
+implementation's own test suite cannot catch a divergence here, because it
+checks that implementation against its own reading of the spec.
+
+This framework aims to become a central place where the materialized value
+expectations of the spec are discussed, so that their representations and
+expectations can be used to verify implementations across languages and
+platforms.
+
+One thing to be clear about up front: we surface ambiguity here, we do not
+settle it here. When a case has no clear answer in the spec, raise it on
+[dev@iceberg.apache.org](https://lists.apache.org/list.html?dev@iceberg.apache.org)
+and leave the unsettled assertion out until there is an clear expectation in the specification to write against. See
Review Comment:
small typo — `an clear` should be `a clear`.
```suggestion
and leave the unsettled assertion out until there is a clear expectation in the specification to write against. See
```
this line also runs well past the wrap width the rest of the doc keeps to — might be worth rewrapping while you're in here.
##########
CONTRIBUTING.md:
##########
@@ -0,0 +1,277 @@
+<!--
+ ~ Licensed to the Apache Software Foundation (ASF) under one
+ ~ or more contributor license agreements. See the NOTICE file
+ ~ distributed with this work for additional information
+ ~ regarding copyright ownership. The ASF licenses this file
+ ~ to you under the Apache License, Version 2.0 (the
+ ~ "License"); you may not use this file except in compliance
+ ~ with the License. You may obtain a copy of the License at
+ ~
+ ~ http://www.apache.org/licenses/LICENSE-2.0
+ ~
+ ~ Unless required by applicable law or agreed to in writing,
+ ~ software distributed under the License is distributed on an
+ ~ "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ ~ KIND, either express or implied. See the License for the
+ ~ specific language governing permissions and limitations
+ ~ under the License.
+ -->
+
+# Contributing
+
+First, thank you for contributing to iceberg-verification! The goal of this
+document is to provide the guidelines you need to maintain high quality
+conformance fixtures for all Iceberg implementations.
+
+[Apache Iceberg](https://iceberg.apache.org/) is first and foremost a specification, an
+agreement between different implementations on the structure and meaning of the
+artifacts that make up a table. And as such, it is crucial that the way each
+implementation interprets the spec, and the values and artifacts it stores and
+transfers as a result, stay consistent across implementations. Each
+implementation's own test suite cannot catch a divergence here, because it
+checks that implementation against its own reading of the spec.
+
+This framework aims to become a central place where the materialized value
+expectations of the spec are discussed, so that their representations and
+expectations can be used to verify implementations across languages and
+platforms.
+
+One thing to be clear about up front: we surface ambiguity here, we do not
+settle it here. When a case has no clear answer in the spec, raise it on
+[dev@iceberg.apache.org](https://lists.apache.org/list.html?dev@iceberg.apache.org)
+and leave the unsettled assertion out until there is an clear expectation in the specification to write against. See
+[Cases the spec has not settled](#cases-the-spec-has-not-settled).
+
+## Does your change belong here?
+
+This repository holds artifacts and the expected values the spec fixes for
+them.
+
+If you have found a bug in one implementation, that belongs in that
+implementation's repository. Open a fixture pull request here when the spec
+fixes an expected valid or rejected value, so that existing and future
+implementations have reference values to verify against.
+
+Engine query results, live catalog protocol behavior, and physical encoding
+choices such as compression codec and file format writer version are out of
+scope. Any spec-valid encoding is valid.
+
+## Choosing a test surface
+
+Given the above goal, it is important that contributors think deeply about what
+we are trying to verify when we introduce a new test surface.
+
+A test surface is one spec behavior verified by one assertion. Each surface
Review Comment:
I think there's a real tension in how a surface gets defined.
This says a surface is "one spec behavior verified by one assertion," but "Cases the spec has not settled" later says "a surface can assert several things about one input, and the spec can fix some and not others." Since surface is the concept the whole doc hangs on, I'd reconcile the two up front — maybe a surface asserts one relationship but can hold several cases, some settled and some open. wdyt?
##########
CONTRIBUTING.md:
##########
@@ -0,0 +1,277 @@
+<!--
+ ~ Licensed to the Apache Software Foundation (ASF) under one
+ ~ or more contributor license agreements. See the NOTICE file
+ ~ distributed with this work for additional information
+ ~ regarding copyright ownership. The ASF licenses this file
+ ~ to you under the Apache License, Version 2.0 (the
+ ~ "License"); you may not use this file except in compliance
+ ~ with the License. You may obtain a copy of the License at
+ ~
+ ~ http://www.apache.org/licenses/LICENSE-2.0
+ ~
+ ~ Unless required by applicable law or agreed to in writing,
+ ~ software distributed under the License is distributed on an
+ ~ "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ ~ KIND, either express or implied. See the License for the
+ ~ specific language governing permissions and limitations
+ ~ under the License.
+ -->
+
+# Contributing
+
+First, thank you for contributing to iceberg-verification! The goal of this
+document is to provide the guidelines you need to maintain high quality
+conformance fixtures for all Iceberg implementations.
+
+[Apache Iceberg](https://iceberg.apache.org/) is first and foremost a specification, an
+agreement between different implementations on the structure and meaning of the
+artifacts that make up a table. And as such, it is crucial that the way each
+implementation interprets the spec, and the values and artifacts it stores and
+transfers as a result, stay consistent across implementations. Each
+implementation's own test suite cannot catch a divergence here, because it
+checks that implementation against its own reading of the spec.
+
+This framework aims to become a central place where the materialized value
+expectations of the spec are discussed, so that their representations and
+expectations can be used to verify implementations across languages and
+platforms.
+
+One thing to be clear about up front: we surface ambiguity here, we do not
+settle it here. When a case has no clear answer in the spec, raise it on
+[dev@iceberg.apache.org](https://lists.apache.org/list.html?dev@iceberg.apache.org)
+and leave the unsettled assertion out until there is an clear expectation in the specification to write against. See
+[Cases the spec has not settled](#cases-the-spec-has-not-settled).
+
+## Does your change belong here?
+
+This repository holds artifacts and the expected values the spec fixes for
+them.
+
+If you have found a bug in one implementation, that belongs in that
+implementation's repository. Open a fixture pull request here when the spec
+fixes an expected valid or rejected value, so that existing and future
+implementations have reference values to verify against.
+
+Engine query results, live catalog protocol behavior, and physical encoding
+choices such as compression codec and file format writer version are out of
+scope. Any spec-valid encoding is valid.
+
+## Choosing a test surface
+
+Given the above goal, it is important that contributors think deeply about what
+we are trying to verify when we introduce a new test surface.
+
+A test surface is one spec behavior verified by one assertion. Each surface
+directory holds its input artifacts, the expected values co-located with them,
+and a `README.md` stating that assertion. Surfaces are grouped by the spec they
+cover, and test surfaces often map to specs. Co-located can mean an
+expected-value file beside each input, or one record carrying both. Where the
+input is small enough to write inline, one record is usually the clearer form.
+
+A surface may be divided into subdirectories along an internal dimension such
+as type or format version. The subdirectories share the surface's assertion and
+its `README.md`. A dimension that changes the assertion is not an internal
+dimension, it is a second surface.
+
+A subdirectory is the unit of subscription. It exists so that a consumer can run
+one directory to answer one conformance question, for example whether its
+implementation supports `variant` columns, and name that directory in its
+own configuration rather than filtering cases at runtime. Taking some
+subdirectories of a surface and not others is normal. Organize them so that the
+choice is expressible by path.
+
+Specs can have dependencies on other Iceberg specs. For example, Table Spec has
+a dependency on Iceberg Type Spec. In such cases, we recommend creating test
+surfaces for both layers at the root level, and focusing on the coverage that
+pertains to each level.
+
+If there are edge case value concerns with the type spec, or we want to verify
+the field structure of a type spec (e.g. Geography Type), we want to include
+that coverage in the surface for the Iceberg Type Spec. Geography Type support
+in a Table V3 spec can be added as a separate test at the Table Spec test
+surface, but the scope of testing will be different. In the Type Spec surface,
+we test edge cases in the type representation. In the Table Spec test, we check
+that V3 supports a representation of Geography Type.
+
+Notice that this is not all that different from how we reason through regular
+test suites. Keep this model in mind when you work on introducing a new test
+surface.
+
+## What should my test fixtures and expectations look like?
+
+There is no single answer here, and that is deliberate. Each combination of
+input fixture and expectation differs by the surface under test, because what
+the spec fixes differs by surface. The spec fixes the canonical form of a type
+string. It pins no such form for `metadata.json`, so two byte-different files
+can represent the same table metadata, and only the decoded values are
+comparable.
+
+It is the contributor's responsibility to understand the spec, work out the
+specification that needs to hold true, and design the input and the assertions
+that define correctness of the materialized values for that spec. Before you
+write a fixture, you should be able to answer:
+
+1. Which clause of which spec fixes the correct answer here? Link it.
+2. What is the input, and what is the smallest complete statement of the
+ expected result?
+3. What must an implementation do to check it, in one sentence? That sentence
+ belongs in the surface `README.md`, and it is the contract consumers
+ implement.
+4. Which boundary cases does the spec decide, and which does it leave open?
+
+That said, a few things hold no matter which surface you are working on.
+
+### Bytes or decoded values?
+
+Where the spec fixes a canonical form, as it does for type strings, we can
+assert that re-serializing the parsed input reproduces that form exactly.
+Everywhere else, we should compare the decoded logical values instead.
+
+It is worth thinking carefully about this, because the stricter option may not
+be the most suitable assertion. If we compare a `metadata.json` or an Avro
+manifest against an expected form byte for byte, we end up freezing one
+implementation's key ordering, and its choices about optional fields and
+codecs, as if the spec had required them. Other implementations would then fail
+our fixture for reasons the spec does not care about, and that is not what we
+want to achieve with the verification framework.
+
+When we compare decoded values, we still need to be careful about what we
+normalize away. Where the spec fixes a logical value but allows more than one
+encoding, we normalize to the value. For example, a `day` transform result is a
+`date`, and the spec requires readers to accept a plain `int` as the same date,
+so the two should compare equal. Where the spec pins a physical type, we keep
+it. `equality_ids` elements are `int`, not `long`, so a decode that reads both
+as `1` would miss the difference.
+
+So the only things we assert are the ones the spec pins. Where the spec allows
+more than one encoding of the same logical value, we normalize and there is no
+difference left to fail on. Where the spec has not decided the answer at all,
+see the next section.
+
+### Cases the spec has not settled
+
+Often the most interesting case is one where the spec does not yet give a clear
+answer, and implementations have landed in different places.
+
+Only what the spec fixes should be asserted. A surface can assert several things
+about one input, and the spec can fix some and not others. An unsettled
+serialized form does not cost a settled parse result. The assertions the spec
+fixes should be written and the rest left out.
+
+If the specification is not clear enough to guide the assertion, the question
+should be raised on
+[dev@iceberg.apache.org](https://lists.apache.org/list.html?dev@iceberg.apache.org)
+and a pull request opened holding the input. The pull request stays open while
+the question is discussed, and it is what the thread points at. It is merged
+once the community settles the answer and the expected value can be written.
+
+Where no discussion has been opened yet, the change or the clause the ambiguity
+originates in should be referenced.
+
+A surface `README.md` names the questions its inputs are waiting on, so the
+boundary is visible at a pinned commit.
+
+### Keep expected values unambiguous across implementations
+
+Your fixture will be read by Go, Python, Rust, Java, C++ consumers and other
+Iceberg implementations that may not be managed by the Apache Iceberg
+community, so the expected value itself must not be ambiguous to read. Say in
+your surface `README.md` how your expected files are written.
+
+Before inventing a convention, look at how existing surfaces handle the same
+problem and follow them where it fits. Once a convention has proven consistent
+across surfaces, we can promote it into this document as a repository-wide
+rule.
+
+#### If your expected values are JSON
+
+- **Record the type wherever the spec pins one.** JSON has a single number
+ type, so `[1]` reads the same whether an implementation decoded an `int` or
+ a `long`. `equality_ids` elements are `int`, and an expected value that
+ recorded only the number would not catch an implementation that decoded them
+ as `long`.
+- **Write 64-bit integers as strings.** Most JSON parsers read numbers as
+ doubles, which are exact only up to 2^53. A `long` value of
+ `3055729675574597004` comes back as `3055729675574597120` after such a parse,
+ with no error raised. Do this for every field the spec types as 64-bit,
+ whatever the value happens to be, so that a field is not a number in one case
+ and a string in another.
+- **Write binary as uppercase hex.** JSON has no binary type. An array of
+ integers diverges immediately, because languages disagree on whether a byte
+ is signed. A Java `byte` is signed and writes `-1` where Go and Python write
+ `255`, and base64 has more than one valid encoding of the same bytes.
+- **Write `null` out rather than leaving the field absent**, so that a reader
+ can tell a value that decoded to null from a field the assertion does not
+ cover.
+
+Override any of these where they do not fit your surface, and say why in its
+`README.md`.
+
+Some values need more than these conventions. For example, the unscaled value
+of a `decimal(38, 10)` does not fit in 64 bits at all, so a surface that covers
Review Comment:
"at all" overstates this — plenty of `decimal(38, 10)` values fit in 64 bits fine (1.0 has unscaled value 10). the real point is that the max unscaled value can overflow.
```suggestion
of a `decimal(38, 10)` can overflow 64 bits, so a surface that covers
```
##########
CONTRIBUTING.md:
##########
@@ -0,0 +1,277 @@
+<!--
+ ~ Licensed to the Apache Software Foundation (ASF) under one
+ ~ or more contributor license agreements. See the NOTICE file
+ ~ distributed with this work for additional information
+ ~ regarding copyright ownership. The ASF licenses this file
+ ~ to you under the Apache License, Version 2.0 (the
+ ~ "License"); you may not use this file except in compliance
+ ~ with the License. You may obtain a copy of the License at
+ ~
+ ~ http://www.apache.org/licenses/LICENSE-2.0
+ ~
+ ~ Unless required by applicable law or agreed to in writing,
+ ~ software distributed under the License is distributed on an
+ ~ "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ ~ KIND, either express or implied. See the License for the
+ ~ specific language governing permissions and limitations
+ ~ under the License.
+ -->
+
+# Contributing
+
+First, thank you for contributing to iceberg-verification! The goal of this
+document is to provide the guidelines you need to maintain high quality
+conformance fixtures for all Iceberg implementations.
+
+[Apache Iceberg](https://iceberg.apache.org/) is first and foremost a specification, an
+agreement between different implementations on the structure and meaning of the
+artifacts that make up a table. And as such, it is crucial that the way each
+implementation interprets the spec, and the values and artifacts it stores and
+transfers as a result, stay consistent across implementations. Each
+implementation's own test suite cannot catch a divergence here, because it
+checks that implementation against its own reading of the spec.
+
+This framework aims to become a central place where the materialized value
+expectations of the spec are discussed, so that their representations and
+expectations can be used to verify implementations across languages and
+platforms.
+
+One thing to be clear about up front: we surface ambiguity here, we do not
+settle it here. When a case has no clear answer in the spec, raise it on
+[dev@iceberg.apache.org](https://lists.apache.org/list.html?dev@iceberg.apache.org)
+and leave the unsettled assertion out until there is an clear expectation in the specification to write against. See
+[Cases the spec has not settled](#cases-the-spec-has-not-settled).
+
+## Does your change belong here?
+
+This repository holds artifacts and the expected values the spec fixes for
+them.
+
+If you have found a bug in one implementation, that belongs in that
+implementation's repository. Open a fixture pull request here when the spec
+fixes an expected valid or rejected value, so that existing and future
+implementations have reference values to verify against.
+
+Engine query results, live catalog protocol behavior, and physical encoding
+choices such as compression codec and file format writer version are out of
+scope. Any spec-valid encoding is valid.
+
+## Choosing a test surface
+
+Given the above goal, it is important that contributors think deeply about what
+we are trying to verify when we introduce a new test surface.
+
+A test surface is one spec behavior verified by one assertion. Each surface
+directory holds its input artifacts, the expected values co-located with them,
+and a `README.md` stating that assertion. Surfaces are grouped by the spec they
+cover, and test surfaces often map to specs. Co-located can mean an
+expected-value file beside each input, or one record carrying both. Where the
+input is small enough to write inline, one record is usually the clearer form.
+
+A surface may be divided into subdirectories along an internal dimension such
+as type or format version. The subdirectories share the surface's assertion and
+its `README.md`. A dimension that changes the assertion is not an internal
+dimension, it is a second surface.
+
+A subdirectory is the unit of subscription. It exists so that a consumer can run
+one directory to answer one conformance question, for example whether its
+implementation supports `variant` columns, and name that directory in its
+own configuration rather than filtering cases at runtime. Taking some
+subdirectories of a surface and not others is normal. Organize them so that the
+choice is expressible by path.
+
+Specs can have dependencies on other Iceberg specs. For example, Table Spec has
+a dependency on Iceberg Type Spec. In such cases, we recommend creating test
+surfaces for both layers at the root level, and focusing on the coverage that
+pertains to each level.
+
+If there are edge case value concerns with the type spec, or we want to verify
+the field structure of a type spec (e.g. Geography Type), we want to include
+that coverage in the surface for the Iceberg Type Spec. Geography Type support
+in a Table V3 spec can be added as a separate test at the Table Spec test
+surface, but the scope of testing will be different. In the Type Spec surface,
+we test edge cases in the type representation. In the Table Spec test, we check
+that V3 supports a representation of Geography Type.
+
+Notice that this is not all that different from how we reason through regular
+test suites. Keep this model in mind when you work on introducing a new test
+surface.
+
+## What should my test fixtures and expectations look like?
+
+There is no single answer here, and that is deliberate. Each combination of
+input fixture and expectation differs by the surface under test, because what
+the spec fixes differs by surface. The spec fixes the canonical form of a type
+string. It pins no such form for `metadata.json`, so two byte-different files
+can represent the same table metadata, and only the decoded values are
+comparable.
+
+It is the contributor's responsibility to understand the spec, work out the
+specification that needs to hold true, and design the input and the assertions
+that define correctness of the materialized values for that spec. Before you
+write a fixture, you should be able to answer:
+
+1. Which clause of which spec fixes the correct answer here? Link it.
+2. What is the input, and what is the smallest complete statement of the
+ expected result?
+3. What must an implementation do to check it, in one sentence? That sentence
+ belongs in the surface `README.md`, and it is the contract consumers
+ implement.
+4. Which boundary cases does the spec decide, and which does it leave open?
+
+That said, a few things hold no matter which surface you are working on.
+
+### Bytes or decoded values?
+
+Where the spec fixes a canonical form, as it does for type strings, we can
+assert that re-serializing the parsed input reproduces that form exactly.
+Everywhere else, we should compare the decoded logical values instead.
+
+It is worth thinking carefully about this, because the stricter option may not
+be the most suitable assertion. If we compare a `metadata.json` or an Avro
+manifest against an expected form byte for byte, we end up freezing one
+implementation's key ordering, and its choices about optional fields and
+codecs, as if the spec had required them. Other implementations would then fail
+our fixture for reasons the spec does not care about, and that is not what we
+want to achieve with the verification framework.
+
+When we compare decoded values, we still need to be careful about what we
+normalize away. Where the spec fixes a logical value but allows more than one
+encoding, we normalize to the value. For example, a `day` transform result is a
+`date`, and the spec requires readers to accept a plain `int` as the same date,
+so the two should compare equal. Where the spec pins a physical type, we keep
+it. `equality_ids` elements are `int`, not `long`, so a decode that reads both
+as `1` would miss the difference.
+
+So the only things we assert are the ones the spec pins. Where the spec allows
+more than one encoding of the same logical value, we normalize and there is no
+difference left to fail on. Where the spec has not decided the answer at all,
+see the next section.
+
+### Cases the spec has not settled
+
+Often the most interesting case is one where the spec does not yet give a clear
+answer, and implementations have landed in different places.
+
+Only what the spec fixes should be asserted. A surface can assert several things
+about one input, and the spec can fix some and not others. An unsettled
+serialized form does not cost a settled parse result. The assertions the spec
+fixes should be written and the rest left out.
+
+If the specification is not clear enough to guide the assertion, the question
+should be raised on
+[dev@iceberg.apache.org](https://lists.apache.org/list.html?dev@iceberg.apache.org)
+and a pull request opened holding the input. The pull request stays open while
+the question is discussed, and it is what the thread points at. It is merged
+once the community settles the answer and the expected value can be written.
+
+Where no discussion has been opened yet, the change or the clause the ambiguity
+originates in should be referenced.
+
+A surface `README.md` names the questions its inputs are waiting on, so the
+boundary is visible at a pinned commit.
+
+### Keep expected values unambiguous across implementations
+
+Your fixture will be read by Go, Python, Rust, Java, C++ consumers and other
+Iceberg implementations that may not be managed by the Apache Iceberg
+community, so the expected value itself must not be ambiguous to read. Say in
+your surface `README.md` how your expected files are written.
+
+Before inventing a convention, look at how existing surfaces handle the same
+problem and follow them where it fits. Once a convention has proven consistent
+across surfaces, we can promote it into this document as a repository-wide
+rule.
+
+#### If your expected values are JSON
+
+- **Record the type wherever the spec pins one.** JSON has a single number
+ type, so `[1]` reads the same whether an implementation decoded an `int` or
+ a `long`. `equality_ids` elements are `int`, and an expected value that
+ recorded only the number would not catch an implementation that decoded them
+ as `long`.
+- **Write 64-bit integers as strings.** Most JSON parsers read numbers as
+ doubles, which are exact only up to 2^53. A `long` value of
+ `3055729675574597004` comes back as `3055729675574597120` after such a parse,
+ with no error raised. Do this for every field the spec types as 64-bit,
+ whatever the value happens to be, so that a field is not a number in one case
+ and a string in another.
+- **Write binary as uppercase hex.** JSON has no binary type. An array of
+ integers diverges immediately, because languages disagree on whether a byte
+ is signed. A Java `byte` is signed and writes `-1` where Go and Python write
+ `255`, and base64 has more than one valid encoding of the same bytes.
+- **Write `null` out rather than leaving the field absent**, so that a reader
+ can tell a value that decoded to null from a field the assertion does not
+ cover.
+
+Override any of these where they do not fit your surface, and say why in its
+`README.md`.
+
+Some values need more than these conventions. For example, the unscaled value
+of a `decimal(38, 10)` does not fit in 64 bits at all, so a surface that covers
+one will need to say how it writes it.
+
+## What we expect of a fixture
+
+- **Derive it from the spec, do not copy it from an implementation.** The
+ implementation you copy from may already share the bug you are trying to
+ catch. Where the spec ships reference vectors, reproduce those first.
+- **Cross-check before you open the pull request.** Confirm the expected value
+ against at least one implementation other than the one that prompted the
+ fixture.
+- **Pin boundary cases, not just the spec's examples.** Drift lives at the
+ edges. Include the cases that must be rejected, not only the ones that must
+ be accepted. A corpus of valid inputs alone cannot catch a parser that is too
+ lenient.
+- **Keep committed files implementation-neutral.** Name the diverging
+ implementation in the pull request, not in the fixture.
+- **Say whether you are adding or correcting.** A consumer bumping its pin
+ needs to know whether a newly failing case is expected.
+
+## Fixture size discipline
+
+Cost is driven by commit count rather than schema width. An Avro manifest has a
+floor near 4.3 KB, since its header embeds the manifest schema, and each further
+snapshot adds a manifest list near 2 KB and a manifest of 4 to 10 KB. Measured
+against PyIceberg 0.12.0 at format version 2, a fifteen-column table with nested
+types over three snapshots comes to 25.7 KB.
+
+A few guidelines to keep fixtures from growing unnecessarily:
+
+- A fixture should be a minimal reproduction of the assertion under test.
+- Snapshot and manifest count should be kept to what that assertion requires.
+ Where an implementation is used to produce a sample table, consider committing
+ only the current `metadata.json` rather than every version the writer produced.
+- Reference tables above 100 KB should be justified in the pull request.
+
+## Opening a pull request
+
+1. Identify the surface. Reuse an existing one where the assertion already
+ fits.
+2. Add the input and its expected value to that surface.
+3. Extend the surface `README.md` if the addition changes what the assertion
+ covers.
+4. In the pull request, state the spec clause the expected value derives from,
+ which implementations you checked it against, and whether the case is an
+ addition or a correction.
+
+A new surface additionally needs a `README.md`. Section names and order are up
+to you, but it has to cover:
+
+- The assertion, stated as an expression and not only as prose.
+- What the surface covers, and what was deliberately left out and why.
+- Each subdirectory, what it covers, and whether a consumer can subscribe to it
+ on its own.
+- The shape of a case, and how to read its expected values.
+- The consumer loop, written so that two implementers cannot read it two ways.
Review Comment:
this is the most implementation-facing thing the doc asks for, and "pseudocode is one way and prose is another" leaves the form open enough that surfaces will end up documenting the loop inconsistently — which is the exact thing this bullet is trying to prevent.
could we pin it down with a short worked example (5-6 lines of pseudocode inline, or in a linked example surface README)? even one concrete loop gives contributors something to copy. Related: the README says "today each consumer writes its own assertions," which reads like this loop is aspirational rather than required — worth making clear which it is. Thoughts?
##########
CONTRIBUTING.md:
##########
@@ -0,0 +1,277 @@
+<!--
+ ~ Licensed to the Apache Software Foundation (ASF) under one
+ ~ or more contributor license agreements. See the NOTICE file
+ ~ distributed with this work for additional information
+ ~ regarding copyright ownership. The ASF licenses this file
+ ~ to you under the Apache License, Version 2.0 (the
+ ~ "License"); you may not use this file except in compliance
+ ~ with the License. You may obtain a copy of the License at
+ ~
+ ~ http://www.apache.org/licenses/LICENSE-2.0
+ ~
+ ~ Unless required by applicable law or agreed to in writing,
+ ~ software distributed under the License is distributed on an
+ ~ "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ ~ KIND, either express or implied. See the License for the
+ ~ specific language governing permissions and limitations
+ ~ under the License.
+ -->
+
+# Contributing
+
+First, thank you for contributing to iceberg-verification! The goal of this
+document is to provide the guidelines you need to maintain high quality
+conformance fixtures for all Iceberg implementations.
+
+[Apache Iceberg](https://iceberg.apache.org/) is first and foremost a specification, an
+agreement between different implementations on the structure and meaning of the
+artifacts that make up a table. And as such, it is crucial that the way each
+implementation interprets the spec, and the values and artifacts it stores and
+transfers as a result, stay consistent across implementations. Each
+implementation's own test suite cannot catch a divergence here, because it
+checks that implementation against its own reading of the spec.
+
+This framework aims to become a central place where the materialized value
+expectations of the spec are discussed, so that their representations and
+expectations can be used to verify implementations across languages and
+platforms.
+
+One thing to be clear about up front: we surface ambiguity here, we do not
+settle it here. When a case has no clear answer in the spec, raise it on
+[dev@iceberg.apache.org](https://lists.apache.org/list.html?dev@iceberg.apache.org)
+and leave the unsettled assertion out until there is an clear expectation in the specification to write against. See
+[Cases the spec has not settled](#cases-the-spec-has-not-settled).
+
+## Does your change belong here?
+
+This repository holds artifacts and the expected values the spec fixes for
+them.
+
+If you have found a bug in one implementation, that belongs in that
+implementation's repository. Open a fixture pull request here when the spec
+fixes an expected valid or rejected value, so that existing and future
+implementations have reference values to verify against.
+
+Engine query results, live catalog protocol behavior, and physical encoding
+choices such as compression codec and file format writer version are out of
+scope. Any spec-valid encoding is valid.
+
+## Choosing a test surface
+
+Given the above goal, it is important that contributors think deeply about what
+we are trying to verify when we introduce a new test surface.
+
+A test surface is one spec behavior verified by one assertion. Each surface
+directory holds its input artifacts, the expected values co-located with them,
+and a `README.md` stating that assertion. Surfaces are grouped by the spec they
+cover, and test surfaces often map to specs. Co-located can mean an
+expected-value file beside each input, or one record carrying both. Where the
+input is small enough to write inline, one record is usually the clearer form.
+
+A surface may be divided into subdirectories along an internal dimension such
+as type or format version. The subdirectories share the surface's assertion and
+its `README.md`. A dimension that changes the assertion is not an internal
+dimension, it is a second surface.
+
+A subdirectory is the unit of subscription. It exists so that a consumer can run
+one directory to answer one conformance question, for example whether its
+implementation supports `variant` columns, and name that directory in its
+own configuration rather than filtering cases at runtime. Taking some
+subdirectories of a surface and not others is normal. Organize them so that the
+choice is expressible by path.
+
+Specs can have dependencies on other Iceberg specs. For example, Table Spec has
+a dependency on Iceberg Type Spec. In such cases, we recommend creating test
Review Comment:
there isn't actually a standalone "Iceberg Type Spec" document — the type system lives in sections of the main table spec (`spec.md`, Primitive Types and Appendix C), alongside view-spec, puffin-spec, rest-catalog-spec, and the others.
this matters because the doc tells contributors to link the spec clause and to group surfaces by the spec they cover, so someone building a type surface will go looking for a spec URL that doesn't exist. I'd refer to it as "the types section of the Iceberg table spec" or point at concrete sections — and it shows up again just below in the type-spec surface references (lines 89-90). If there's active work to split types into its own spec doc, worth saying so explicitly.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Add root level `CONTRIBUTING.md` [iceberg-verification]
Posted by "sungwy (via GitHub)" <gi...@apache.org>.
sungwy commented on code in PR #2:
URL: https://github.com/apache/iceberg-verification/pull/2#discussion_r4028924316
##########
CONTRIBUTING.md:
##########
@@ -0,0 +1,277 @@
+<!--
+ ~ Licensed to the Apache Software Foundation (ASF) under one
+ ~ or more contributor license agreements. See the NOTICE file
+ ~ distributed with this work for additional information
+ ~ regarding copyright ownership. The ASF licenses this file
+ ~ to you under the Apache License, Version 2.0 (the
+ ~ "License"); you may not use this file except in compliance
+ ~ with the License. You may obtain a copy of the License at
+ ~
+ ~ http://www.apache.org/licenses/LICENSE-2.0
+ ~
+ ~ Unless required by applicable law or agreed to in writing,
+ ~ software distributed under the License is distributed on an
+ ~ "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ ~ KIND, either express or implied. See the License for the
+ ~ specific language governing permissions and limitations
+ ~ under the License.
+ -->
+
+# Contributing
+
+First, thank you for contributing to iceberg-verification! The goal of this
+document is to provide the guidelines you need to maintain high quality
+conformance fixtures for all Iceberg implementations.
+
+[Apache Iceberg](https://iceberg.apache.org/) is first and foremost a specification, an
+agreement between different implementations on the structure and meaning of the
+artifacts that make up a table. And as such, it is crucial that the way each
+implementation interprets the spec, and the values and artifacts it stores and
+transfers as a result, stay consistent across implementations. Each
+implementation's own test suite cannot catch a divergence here, because it
+checks that implementation against its own reading of the spec.
+
+This framework aims to become a central place where the materialized value
+expectations of the spec are discussed, so that their representations and
+expectations can be used to verify implementations across languages and
+platforms.
+
+One thing to be clear about up front: we surface ambiguity here, we do not
+settle it here. When a case has no clear answer in the spec, raise it on
+[dev@iceberg.apache.org](https://lists.apache.org/list.html?dev@iceberg.apache.org)
+and leave the unsettled assertion out until there is an clear expectation in the specification to write against. See
+[Cases the spec has not settled](#cases-the-spec-has-not-settled).
+
+## Does your change belong here?
+
+This repository holds artifacts and the expected values the spec fixes for
+them.
+
+If you have found a bug in one implementation, that belongs in that
+implementation's repository. Open a fixture pull request here when the spec
+fixes an expected valid or rejected value, so that existing and future
+implementations have reference values to verify against.
+
+Engine query results, live catalog protocol behavior, and physical encoding
+choices such as compression codec and file format writer version are out of
+scope. Any spec-valid encoding is valid.
+
+## Choosing a test surface
+
+Given the above goal, it is important that contributors think deeply about what
+we are trying to verify when we introduce a new test surface.
+
+A test surface is one spec behavior verified by one assertion. Each surface
+directory holds its input artifacts, the expected values co-located with them,
+and a `README.md` stating that assertion. Surfaces are grouped by the spec they
+cover, and test surfaces often map to specs. Co-located can mean an
+expected-value file beside each input, or one record carrying both. Where the
+input is small enough to write inline, one record is usually the clearer form.
+
+A surface may be divided into subdirectories along an internal dimension such
+as type or format version. The subdirectories share the surface's assertion and
+its `README.md`. A dimension that changes the assertion is not an internal
+dimension, it is a second surface.
+
+A subdirectory is the unit of subscription. It exists so that a consumer can run
+one directory to answer one conformance question, for example whether its
+implementation supports `variant` columns, and name that directory in its
+own configuration rather than filtering cases at runtime. Taking some
+subdirectories of a surface and not others is normal. Organize them so that the
+choice is expressible by path.
+
+Specs can have dependencies on other Iceberg specs. For example, Table Spec has
+a dependency on Iceberg Type Spec. In such cases, we recommend creating test
+surfaces for both layers at the root level, and focusing on the coverage that
+pertains to each level.
+
+If there are edge case value concerns with the type spec, or we want to verify
+the field structure of a type spec (e.g. Geography Type), we want to include
+that coverage in the surface for the Iceberg Type Spec. Geography Type support
+in a Table V3 spec can be added as a separate test at the Table Spec test
+surface, but the scope of testing will be different. In the Type Spec surface,
+we test edge cases in the type representation. In the Table Spec test, we check
+that V3 supports a representation of Geography Type.
+
+Notice that this is not all that different from how we reason through regular
+test suites. Keep this model in mind when you work on introducing a new test
+surface.
+
+## What should my test fixtures and expectations look like?
+
+There is no single answer here, and that is deliberate. Each combination of
+input fixture and expectation differs by the surface under test, because what
+the spec fixes differs by surface. The spec fixes the canonical form of a type
+string. It pins no such form for `metadata.json`, so two byte-different files
+can represent the same table metadata, and only the decoded values are
+comparable.
+
+It is the contributor's responsibility to understand the spec, work out the
+specification that needs to hold true, and design the input and the assertions
+that define correctness of the materialized values for that spec. Before you
+write a fixture, you should be able to answer:
+
+1. Which clause of which spec fixes the correct answer here? Link it.
+2. What is the input, and what is the smallest complete statement of the
+ expected result?
+3. What must an implementation do to check it, in one sentence? That sentence
+ belongs in the surface `README.md`, and it is the contract consumers
+ implement.
+4. Which boundary cases does the spec decide, and which does it leave open?
+
+That said, a few things hold no matter which surface you are working on.
+
+### Bytes or decoded values?
+
+Where the spec fixes a canonical form, as it does for type strings, we can
+assert that re-serializing the parsed input reproduces that form exactly.
+Everywhere else, we should compare the decoded logical values instead.
+
+It is worth thinking carefully about this, because the stricter option may not
+be the most suitable assertion. If we compare a `metadata.json` or an Avro
+manifest against an expected form byte for byte, we end up freezing one
+implementation's key ordering, and its choices about optional fields and
+codecs, as if the spec had required them. Other implementations would then fail
+our fixture for reasons the spec does not care about, and that is not what we
+want to achieve with the verification framework.
+
+When we compare decoded values, we still need to be careful about what we
+normalize away. Where the spec fixes a logical value but allows more than one
+encoding, we normalize to the value. For example, a `day` transform result is a
+`date`, and the spec requires readers to accept a plain `int` as the same date,
+so the two should compare equal. Where the spec pins a physical type, we keep
+it. `equality_ids` elements are `int`, not `long`, so a decode that reads both
+as `1` would miss the difference.
+
+So the only things we assert are the ones the spec pins. Where the spec allows
+more than one encoding of the same logical value, we normalize and there is no
+difference left to fail on. Where the spec has not decided the answer at all,
+see the next section.
+
+### Cases the spec has not settled
+
+Often the most interesting case is one where the spec does not yet give a clear
+answer, and implementations have landed in different places.
+
+Only what the spec fixes should be asserted. A surface can assert several things
+about one input, and the spec can fix some and not others. An unsettled
+serialized form does not cost a settled parse result. The assertions the spec
+fixes should be written and the rest left out.
+
+If the specification is not clear enough to guide the assertion, the question
+should be raised on
+[dev@iceberg.apache.org](https://lists.apache.org/list.html?dev@iceberg.apache.org)
+and a pull request opened holding the input. The pull request stays open while
+the question is discussed, and it is what the thread points at. It is merged
+once the community settles the answer and the expected value can be written.
+
+Where no discussion has been opened yet, the change or the clause the ambiguity
+originates in should be referenced.
+
+A surface `README.md` names the questions its inputs are waiting on, so the
+boundary is visible at a pinned commit.
+
+### Keep expected values unambiguous across implementations
+
+Your fixture will be read by Go, Python, Rust, Java, C++ consumers and other
+Iceberg implementations that may not be managed by the Apache Iceberg
+community, so the expected value itself must not be ambiguous to read. Say in
+your surface `README.md` how your expected files are written.
+
+Before inventing a convention, look at how existing surfaces handle the same
+problem and follow them where it fits. Once a convention has proven consistent
+across surfaces, we can promote it into this document as a repository-wide
+rule.
+
+#### If your expected values are JSON
+
+- **Record the type wherever the spec pins one.** JSON has a single number
+ type, so `[1]` reads the same whether an implementation decoded an `int` or
+ a `long`. `equality_ids` elements are `int`, and an expected value that
+ recorded only the number would not catch an implementation that decoded them
+ as `long`.
+- **Write 64-bit integers as strings.** Most JSON parsers read numbers as
+ doubles, which are exact only up to 2^53. A `long` value of
+ `3055729675574597004` comes back as `3055729675574597120` after such a parse,
+ with no error raised. Do this for every field the spec types as 64-bit,
+ whatever the value happens to be, so that a field is not a number in one case
+ and a string in another.
+- **Write binary as uppercase hex.** JSON has no binary type. An array of
+ integers diverges immediately, because languages disagree on whether a byte
+ is signed. A Java `byte` is signed and writes `-1` where Go and Python write
+ `255`, and base64 has more than one valid encoding of the same bytes.
+- **Write `null` out rather than leaving the field absent**, so that a reader
+ can tell a value that decoded to null from a field the assertion does not
+ cover.
+
+Override any of these where they do not fit your surface, and say why in its
+`README.md`.
+
+Some values need more than these conventions. For example, the unscaled value
+of a `decimal(38, 10)` does not fit in 64 bits at all, so a surface that covers
+one will need to say how it writes it.
+
+## What we expect of a fixture
+
+- **Derive it from the spec, do not copy it from an implementation.** The
+ implementation you copy from may already share the bug you are trying to
+ catch. Where the spec ships reference vectors, reproduce those first.
+- **Cross-check before you open the pull request.** Confirm the expected value
+ against at least one implementation other than the one that prompted the
+ fixture.
+- **Pin boundary cases, not just the spec's examples.** Drift lives at the
+ edges. Include the cases that must be rejected, not only the ones that must
+ be accepted. A corpus of valid inputs alone cannot catch a parser that is too
+ lenient.
+- **Keep committed files implementation-neutral.** Name the diverging
+ implementation in the pull request, not in the fixture.
+- **Say whether you are adding or correcting.** A consumer bumping its pin
+ needs to know whether a newly failing case is expected.
+
+## Fixture size discipline
+
+Cost is driven by commit count rather than schema width. An Avro manifest has a
+floor near 4.3 KB, since its header embeds the manifest schema, and each further
+snapshot adds a manifest list near 2 KB and a manifest of 4 to 10 KB. Measured
+against PyIceberg 0.12.0 at format version 2, a fifteen-column table with nested
+types over three snapshots comes to 25.7 KB.
+
+A few guidelines to keep fixtures from growing unnecessarily:
+
+- A fixture should be a minimal reproduction of the assertion under test.
+- Snapshot and manifest count should be kept to what that assertion requires.
+ Where an implementation is used to produce a sample table, consider committing
+ only the current `metadata.json` rather than every version the writer produced.
+- Reference tables above 100 KB should be justified in the pull request.
+
+## Opening a pull request
+
+1. Identify the surface. Reuse an existing one where the assertion already
+ fits.
+2. Add the input and its expected value to that surface.
+3. Extend the surface `README.md` if the addition changes what the assertion
+ covers.
+4. In the pull request, state the spec clause the expected value derives from,
+ which implementations you checked it against, and whether the case is an
+ addition or a correction.
+
+A new surface additionally needs a `README.md`. Section names and order are up
+to you, but it has to cover:
+
+- The assertion, stated as an expression and not only as prose.
+- What the surface covers, and what was deliberately left out and why.
+- Each subdirectory, what it covers, and whether a consumer can subscribe to it
+ on its own.
+- The shape of a case, and how to read its expected values.
+- The consumer loop, written so that two implementers cannot read it two ways.
Review Comment:
Maybe? my goal was to add the examples and have the examples speak for itself. How about we enhance this part of the documentation if we observe that contributors find themselves to be confused when it comes to defining a new assertion?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Add root level `CONTRIBUTING.md` [iceberg-verification]
Posted by "sungwy (via GitHub)" <gi...@apache.org>.
sungwy commented on PR #2:
URL: https://github.com/apache/iceberg-verification/pull/2#issuecomment-5706422702
Thanks @nssalian and @laskoviymishka for the reviews
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Add root level `CONTRIBUTING.md` [iceberg-verification]
Posted by "sungwy (via GitHub)" <gi...@apache.org>.
sungwy merged PR #2:
URL: https://github.com/apache/iceberg-verification/pull/2
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org