c-thiel commented on code in PR #331:
URL: https://github.com/apache/iceberg-rust/pull/331#discussion_r1670513358


##########
crates/iceberg/testdata/view_metadata/ViewMetadataV2Valid.json:
##########
@@ -0,0 +1,58 @@
+{

Review Comment:
   @nastra I added more tests that cover non-happy cases. 
   However, there isn't much in 
https://github.com/apache/iceberg/tree/5ea78e3fbe5d8c9c846a1b86bcd2b77d13a31acd/core/src/test/java/org/apache/iceberg/view
 we can use for now. The `ViewMetadataBuilder` I started with is on the same 
level as the `TableMetadataBuilder` - it can do almost nothing except changing 
the UUID. The functions tested in the java part are simply not implemented yet. 
We do have a more complete implementation over at our Iceberg catalog and want 
to PR them here (https://github.com/hansetag/iceberg-catalog/issues/46), I just 
want to start with the foundation before heading into the more controversial 
builder bits.
   Would this be alright for you?
   
   Besides the Metadata-Builder, I would like to create 2 follow-up issues:
   - Currently in `Snapshots` we use a fallible method for the timestamp() 
method. For views I am using a non-fallible version, creating a small 
inconsistency. I would create a new issues to remove `unwrap` from timestamps 
where they are used.
   - Allow for empty namespaces, following our discussion below. I am not sure 
exactly how to handle this, but right now Rust does not allow the creation of 
the empty Namespace as used in spark 
(https://github.com/apache/iceberg-rust/blob/48f9e3e8bff0224569da8dd5beb89ed0c8cc4513/crates/iceberg/src/catalog/mod.rs#L111).



-- 
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: issues-unsubscr...@iceberg.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscr...@iceberg.apache.org
For additional commands, e-mail: issues-h...@iceberg.apache.org

Reply via email to