tanmayrauth commented on PR #963:
URL: https://github.com/apache/iceberg-go/pull/963#issuecomment-4366803973
Thanks for the thorough review, these are all fair points and I agree it's
better to nail the base contract before the follow-ups build on it.
Here's my plan to address each concern in this PR:
1. Namespace properties : Java's HadoopCatalog also doesn't persist
namespace properties, createNamespace rejects non-empty properties with
UnsupportedOperationException, and setProperties/removeProperties both throw
UnsupportedOperationException. loadNamespaceMetadata returns only a
computed {"location": path}. My implementation already matches this, but I'll
add explicit documentation and make sure UpdateNamespaceProperties returns a
clear "not supported" error to match Java's contract.
2. Location format — I looked into this and Java's
HadoopCatalog.loadNamespaceMetadata returns nsPath.toString() which for local
filesystem is a bare POSIX path, not a file:// URI. That said, I agree file://
is more correct as a URI, so I'll switch to file:// locations for consistency
with the broader Iceberg ecosystem.
3. CreateNamespace : Stat → MkdirAll race - I'll replace the Stat +
MkdirAll with a single os.Mkdir call, which fails atomically if the directory
already exists. This also enforces that parent namespaces must exist first,
matching Java's fs.mkdirs behavior for single-level creation.
4. Path validation : I'll add central validation for identifier
components: reject .., empty strings, and path separators. Will add this as a
shared helper so the follow-up PRs get it for free.
5. isTableDir interop : I'll broaden the check to also match UUID-style
metadata filenames (00000-<uuid>.metadata.json) and version-hint.text, so we
correctly detect tables created by Java/PyIceberg.
I'll update this PR with these changes.
--
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]
