LuciferYang commented on PR #2865:
URL: https://github.com/apache/iceberg-rust/pull/2865#issuecomment-5044624143
The position-delete-file internal column names `pos` and `file_path` were
registered in the same reserved-name lookup (`get_metadata_field_id`) that
`is_metadata_column_name` consults, so scan planning treated a user data column
named `pos`/`file_path` as a metadata column: it resolved to a reserved field
id and was never found in the file.
The fix guards `is_metadata_column_name` with `starts_with('_')`. Iceberg
reserves metadata columns by a high field-id range, and by convention their
names are `_`-prefixed; `pos` and `file_path` are the only reserved names in
our lookup without that prefix, so the guard excludes exactly those two.
Iceberg Java draws the same line for these two names. In
`core/.../MetadataColumns.java`, `DELETE_FILE_PATH`/`DELETE_FILE_POS` are the
bare names `"file_path"`/`"pos"` (ids `Integer.MAX_VALUE - 101`/`- 102`), but
they are not in the `META_COLUMNS` map that backs `isMetadataColumn(String)`,
so `isMetadataColumn("pos")` is false and a `pos` data column reads normally
(`nonMetadataColumn("pos")` is true). Java uses an explicit `META_COLUMNS`
allowlist rather than a prefix check, so this isn't a one-to-one port: our
lookup also keeps the changelog names (`_change_type`, etc.) that Java's map
omits. The match is in the observable result for `pos`/`file_path`, not the
mechanism.
Does this line up with the intent behind keeping `file_path`/`pos` out of
`META_COLUMNS`?
@CTTY @mbutrovich @hsiang-c a review would be welcome when you have time.
This is adjacent to the `_pos` work in #2746 but orthogonal: `_pos` stays a
metadata column, while a bare `pos` becomes a normal data column again. I'd
also like your read on whether the deeper cleanup (dropping the two bare names
from the nameāid map) belongs here or in a follow-up.
cc @wjixiang, who reported #2837.
--
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]
