LuciferYang opened a new pull request, #2865: URL: https://github.com/apache/iceberg-rust/pull/2865
## Which issue does this PR close? - Closes #2837. ## What changes are included in this PR? A table with a user data column named `pos` (or `file_path`) writes fine and shows up correctly in the catalog schema, but any scan that projects it fails with `field not found`. Renaming the column makes it readable again. This is a real problem for e.g. VCF → Iceberg pipelines, where `pos` is one of the most common column names. Root cause: the bare names `pos` and `file_path` are the internal columns of a *position-delete file*, but they were registered in the same reserved name→field-id map (`get_metadata_field_id`) that a data-table scan consults via `is_metadata_column_name`. Scan planning resolves metadata column names with metadata-first precedence and no fallback to the data schema (`scan/mod.rs`), so a real data column named `pos`/`file_path` is intercepted by the metadata branch, mapped to a reserved delete-file id, and never looked up in the actual schema. Per the [Iceberg spec](https://iceberg.apache.org/spec/#reserved-field-ids), reserved metadata column names are `_`-prefixed precisely so they cannot collide with user data columns (`_file`, `_pos`, `_partition`, `_spec_id`, `_deleted`). The two bare delete-file names are the only exceptions. This PR guards `is_metadata_column_name` with a `starts_with('_')` check, so those two names are no longer treated as projectable data-table metadata columns. This mirrors the Java reference implementation, which reads a data column named `pos` normally. `get_metadata_field_id` is intentionally left mapping the bare names to their reserved ids: the position-delete read path depends on the id→field direction (`get_metadata_field` is used by `record_batch_transformer` and manifest bounds deserialization). Its doc now notes it is not a membership test — use `is_metadata_column_name` for that. No public API signature changes, so `public-api.txt` is unchanged. ### An open design question for reviewers This is the minimal, low-risk fix: it only stops the *scan* consumer from treating the bare names as metadata columns. The root cause — the two names living in the shared name→id map at all — is left in place, which leaves `get_metadata_field_id("pos").is_ok()` still returning `true` (that is exactly this function's pre-fix body, and the most intuitive way to write a "is this reserved?" check), and leaves the two public predicates disagreeing (`is_metadata_column_name("pos") == false` vs `get_metadata_field_id("pos") == Ok(..)`). A more thorough fix would remove/isolate those two `pos`/`file_path` arms from the name→id map. I believe that is safe in-repo (the only in-repo consumer of the returned id is the now-guarded `scan` call site; the delete read path uses the id→field direction, which would be untouched), but it changes the observable behavior of a second public function, so I kept it out of this PR. Happy to fold it in if you'd prefer the deeper fix — wanted to check the preference first. ## Are these changes tested? Yes, unit tests only (no infrastructure needed): - `metadata_columns`: `is_metadata_column_name` accepts the `_`-prefixed names, rejects the bare `pos`/`file_path` and ordinary data columns, while `get_metadata_field_id` still maps the bare names to their reserved ids. - `scan`: projecting a data column named `pos`/`file_path` — both explicitly (`select`) and via the default all-columns projection — resolves to its real field id instead of the reserved delete-file id; projecting a genuinely absent column still fails with a clear `DataInvalid` / "not found" error. Verified these fail on the pre-fix code and pass with the fix. Validated with: - `cargo test -p iceberg --lib` (1430 passed) - `cargo fmt --all -- --check` - `cargo clippy -p iceberg --all-targets --all-features -- -D warnings` -- 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]
