mfellner opened a new issue, #3403:
URL: https://github.com/apache/iceberg-rust/issues/3403
Related to #2411 (v3 tracking). Related to #3362 (name-based resolution of
metadata columns, a different mechanism).
### Apache Iceberg Rust version
`main` at a76678363
### Describe the bug
The spec's Reserved Field IDs section says: "Iceberg tables must not use
field ids greater than 2147483447 (`Integer.MAX_VALUE - 200`). This id range is
reserved for metadata columns that can be used in user data schemas, like the
`_file` column that holds the file path in which a row was stored." The `_file`
column has id 2147483646 (`RESERVED_FIELD_ID_FILE` in
`crates/iceberg/src/metadata_columns.rs`).
Table metadata does not enforce this limit.
`TableMetadataBuilder::add_schema` and JSON deserialization of table metadata
both accept a schema with field id 2147483448, the first id above the limit,
for format versions v2 and v3. The boundary id 2147483447 is accepted, as the
spec allows.
The reader matches columns by field id, so a user column that uses a
metadata id is read as that metadata column
(`crates/iceberg/src/arrow/reader/pipeline.rs:326`,
`crates/iceberg/src/arrow/record_batch_transformer.rs:734`). In a local probe,
a Parquet file with a column `payload` (field id 2147483646) and a table schema
with the same id returned the data file path for all three rows instead of the
stored values `a`, `b`, `c`. This state is reachable only from table metadata
that the spec forbids, which the check requested below would reject.
I would expect the check to sit at the table metadata level rather than in
`SchemaBuilder::build`, because the crate builds internal schemas that use
reserved ids, such as the position delete schema in
`crates/iceberg/src/writer/base_writer/position_delete_writer.rs`.
### To Reproduce
On `main` at a76678363, add this test module at the end of
`crates/iceberg/src/spec/table_metadata_builder.rs`:
<details>
<summary>Probe test module</summary>
```rust
#[cfg(test)]
mod reserved_id_probe {
use std::collections::HashMap;
use crate::spec::*;
fn base_metadata(format_version: FormatVersion) -> TableMetadata {
let schema = Schema::builder()
.with_fields(vec![NestedField::required(1, "x",
Type::Primitive(PrimitiveType::Long)).into()])
.build()
.unwrap();
TableMetadataBuilder::new(
schema,
UnboundPartitionSpec::builder().build(),
SortOrder::unsorted_order(),
"s3://bucket/t".to_string(),
format_version,
HashMap::new(),
)
.unwrap()
.build()
.unwrap()
.metadata
}
#[test]
fn probe_add_schema_field_ids() {
for format_version in [FormatVersion::V2, FormatVersion::V3] {
for id in [2147483447, 2147483448] {
let schema = Schema::builder()
.with_fields(vec![NestedField::optional(id, "user_col",
Type::Primitive(PrimitiveType::String)).into()])
.build()
.unwrap();
let result = base_metadata(format_version)
.into_builder(Some("s3://bucket/t/metadata/v1.json".to_string()))
.add_schema(schema)
.and_then(|b| b.build());
println!("PROBE add_schema format={format_version:?} id={id}
is_ok={}", result.is_ok());
}
}
}
#[test]
fn probe_metadata_json_field_ids() {
for format_version in [FormatVersion::V2, FormatVersion::V3] {
for id in [2147483447, 2147483448] {
let mut value =
serde_json::to_value(base_metadata(format_version)).unwrap();
value["schemas"][0]["fields"][0]["id"] =
serde_json::json!(id);
let result = serde_json::from_value::<TableMetadata>(value);
println!("PROBE metadata_json format={format_version:?}
id={id} is_ok={}", result.is_ok());
}
}
}
}
```
</details>
Run `cargo test -p iceberg --lib table_metadata_builder::reserved_id_probe
-- --nocapture`. Output:
```
PROBE add_schema format=V2 id=2147483447 is_ok=true
PROBE add_schema format=V2 id=2147483448 is_ok=true
PROBE add_schema format=V3 id=2147483447 is_ok=true
PROBE add_schema format=V3 id=2147483448 is_ok=true
PROBE metadata_json format=V2 id=2147483447 is_ok=true
PROBE metadata_json format=V2 id=2147483448 is_ok=true
PROBE metadata_json format=V3 id=2147483447 is_ok=true
PROBE metadata_json format=V3 id=2147483448 is_ok=true
```
Reader case, on the same tree: a Parquet file with `id` (field id 1, Int32)
and `payload` (Utf8, field id 2147483646) holding `a`, `b`, `c`. The table
schema has `id` (id 1) and an optional `payload` at id 2147483646. The scan
projects field ids `[1, 2147483646]`. This came from a local probe test of
about 60 lines that is not part of the upstream tree, so it is not included
here. Output:
```
PROBE_SCHEMA [("id", "Int32"), ("_file", "RunEndEncoded(\"run_ends\":
non-null Int32, \"values\": Utf8)")]
PROBE_RESULT payload_values=["<data file path>", "<data file path>", "<data
file path>"]
```
### Expected behavior
Adding a schema with a field id greater than 2147483447 to table metadata
(`add_schema`), and parsing table metadata JSON with such a schema, return an
error, as the spec requires. The boundary id 2147483447 stays valid.
### Willingness to contribute
I would be willing to contribute a fix with guidance from the Iceberg
community (any contribution would be agent-assisted and I would disclose that).
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]
