u70b3 commented on PR #2754:
URL: https://github.com/apache/iceberg-rust/pull/2754#issuecomment-4873979031
Could we clarify the exact semantics of `FileIO::list_prefix` in the API
docs, and ideally add a regression test for the edge case where the input
points to an existing file?
I did a quick local check on this PR head by adding temporary tests for
`MemoryStorage` and `LocalFsStorage`:
- write `memory://warehouse/t/data/a.parquet`
- call `list_prefix("memory://warehouse/t/data/a.parquet")`
- result is empty
The same is true for local fs: if the input path is an existing file,
`LocalFsStorage::list_prefix` returns an empty list because it first checks
`!root.is_dir()`. For memory, the implementation normalizes a
non-trailing-slash path into `path/`, so an exact file key does not match
either. OpenDAL appears to follow the same directory-style behavior by listing
`relative_path/`.
That behavior may be exactly what we want for the intended first consumer,
orphan-file cleanup, because the operation is normally scoped to a table
location or a data/metadata directory. This also lines up with the Iceberg
docs/spec side: the table metadata `location` is the table's base location used
for data files, manifests, and metadata files; maintenance docs describe orphan
cleanup as removing files "under a table location", and Spark's
`remove_orphan_files` `location` argument is documented as a directory to look
for files in.
However, the name `list_prefix` can also be read as raw object-key prefix
matching. I cross-checked the Java API and it is intentionally explicit about
this ambiguity: `SupportsPrefixOperations.listPrefix` says hierarchical file
systems may require the prefix to fully match a directory, while key/value
object stores may allow arbitrary prefixes. Java's orphan-file listing path
also normalizes the directory by appending `/` before calling `listPrefix`,
which suggests directory/table-location traversal is the expected maintenance
use case. On the other hand, Java `HadoopFileIO` can return a single file for
an exact file path via `FileSystem.listFiles(path, true)`, so Rust's current
local/memory behavior is not exactly the same for that edge case.
So I think either behavior can be defensible, but the contract should be
explicit. Could we document whether `list_prefix` is intended to:
1. list files under a directory/table-location prefix only, excluding an
exact file-path match, or
2. behave like raw prefix matching / Java-style `listPrefix`, where an exact
file path may be returned by some implementations?
If the current directory-style semantics are intentional, a small test
asserting that listing an exact file path returns empty would make the contract
much clearer for future orphan-file cleanup callers.
--
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]
