mfellner commented on issue #3373:
URL: https://github.com/apache/iceberg-rust/issues/3373#issuecomment-6112672628
Related to #2411 (v3 tracking).
While checking `unknown` against the spec I found one validation gap that
may belong on this tracker, in case it is useful. Checked on main at a76678363.
**Required `unknown` fields are accepted**
The v3 spec's Primitive Types table defines `unknown` as "Default / null
column type used when a more specific type is not known", with the requirement
"Must be optional with `null` defaults" (`format/spec.md` in apache/iceberg,
Primitive Types table).
Iceberg Rust accepts a required field of type `unknown` in all three
construction paths I checked:
- `NestedField::required` passed to `Schema::builder().build()`
- `serde_json::from_value::<Schema>` on a schema containing a required
`unknown` field
- `arrow_schema_to_schema` on a non-nullable `DataType::Null` field
An optional `unknown` field is accepted by the builder (control case in the
test below).
A review of #2773 on 2026-09-15 noted that nothing prevents a required
`unknown` field. The PR was merged on 2026-09-17, and the merged code has no
such check.
Code reading on main at a76678363 (not executed separately):
- `NestedField::required` sets `required: true` without looking at the type
(`crates/iceberg/src/spec/datatypes.rs:681-683`).
- `SchemaBuilder::build` (`crates/iceberg/src/spec/schema/mod.rs:139`) has
no check on required `unknown` fields.
- `arrow_schema_to_schema` sets `required: !field.is_nullable()`
(`crates/iceberg/src/arrow/schema.rs:392`) and maps `DataType::Null` to
`Unknown` (`crates/iceberg/src/arrow/schema.rs:486`).
- The only `unknown` check on the default path rejects non-null defaults
(`crates/iceberg/src/spec/values/literal.rs:524-526`). It does not look at the
`required` flag.
**To reproduce**
Add the file below as `crates/iceberg/tests/probe_required_unknown.rs` and
run:
```
cargo test --offline -p iceberg --test probe_required_unknown -- --nocapture
```
<details>
<summary>probe_required_unknown.rs</summary>
```rust
use std::collections::HashMap;
use arrow_schema::{DataType, Field, Schema as ArrowSchema};
use iceberg::arrow::arrow_schema_to_schema;
use iceberg::spec::{NestedField, PrimitiveType, Schema, Type};
#[test]
fn builder_accepts_required_unknown_field() {
let field = NestedField::required(1, "u",
Type::Primitive(PrimitiveType::Unknown));
println!("NestedField::required(unknown).required = {}", field.required);
let result = Schema::builder()
.with_fields(vec![field.into()])
.build();
println!("SchemaBuilder::build -> {:?}", result.as_ref().map(|s|
s.as_struct().to_string()));
assert!(result.is_ok());
}
#[test]
fn serde_accepts_required_unknown_field() {
let schema_json = serde_json::json!({
"type": "struct",
"schema-id": 0,
"fields": [{"id": 1, "name": "u", "required": true, "type":
"unknown"}]
});
let result = serde_json::from_value::<Schema>(schema_json);
println!("serde Schema::from_value -> is_ok={}", result.is_ok());
assert!(result.is_ok());
}
#[test]
fn arrow_accepts_non_nullable_null_column() {
let md = HashMap::from([("PARQUET:field_id".to_string(),
"1".to_string())]);
let arrow = ArrowSchema::new(vec![Field::new("u", DataType::Null,
false).with_metadata(md)]);
let result = arrow_schema_to_schema(&arrow);
println!("arrow_schema_to_schema(non-nullable Null) -> {:?}",
result.as_ref().map(|s| s.as_struct().to_string()));
let schema = result.expect("conversion succeeds");
println!("field required = {}", schema.as_struct().fields()[0].required);
assert!(schema.as_struct().fields()[0].required);
}
#[test]
fn optional_unknown_is_accepted_control() {
let result = Schema::builder()
.with_fields(vec![NestedField::optional(1, "u",
Type::Primitive(PrimitiveType::Unknown)).into()])
.build();
assert!(result.is_ok());
}
```
</details>
Output on main at a76678363 (Rust `nightly-2026-04-16` from
`rust-toolchain.toml`):
```
NestedField::required(unknown).required = true
SchemaBuilder::build -> Ok("struct<unknown>")
arrow_schema_to_schema(non-nullable Null) -> Ok("struct<unknown>")
field required = true
serde Schema::from_value -> is_ok=true
test result: ok. 4 passed; 0 failed
```
The first three tests pass because the required `unknown` field is accepted.
The fourth is the optional control.
**Expected**
`NestedField::required` with type `unknown`, `SchemaBuilder::build`, and
`arrow_schema_to_schema` should return an error for a required `unknown` field.
Optional `unknown` fields should remain accepted.
How existing required `unknown` fields should be handled on deserialization
is left open here. The load-time vs construction-time validation question in
#3250 is still under discussion.
Found during an agent-assisted audit of v3 support (see #2411). The
reproduction above was run against main at a76678363.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
