c-thiel commented on PR #645:
URL: https://github.com/apache/iceberg-rust/pull/645#issuecomment-2442922625

   @Xuanwo, @liurenjie1024 ready for another round.
   Ditched both the trait and the enum. Bare with me:
   
   As already mentioned earlier, the enum or the trait were just a vehicle to 
reduce duplication of logic and to clarify that there are three variants of 
partition specs with different nuances. However the enum should rarely be used 
directly, as individual variants are much more powerful with a clear hierarchy: 
Bound > Schemaless > Unbound. Thus, it would be bad to ship the enum around to 
functions that require the Bound or Schemaless variants (i.e. need a field_id). 
It would always require matching and error handling. Thus, we use the specific 
variants pretty much everywhere anyway.
   
   If we agree that we shouldn't use the enum to pass PartitionSpecs, then 
having an enum with owned fields is bad, because it requires a lot of `clone`. 
This is why I oped for `enum<'a> PartitionSpec<'a>` in 
https://github.com/apache/iceberg-rust/commit/9562d22a2cc550ecaf04cb857b0ad15e67c94603.
   
   I didn't like the enum though as I didn't want to use it anyway. So in my 
last commit I got rid of that as well.
   We gain a lot less code and API surface. We loose quite a bit of 
interoperability between different schema variants. What especially hurts is 
the `is_compatible_with` method, which used to be completely cross-functional 
for all variants.
   I implement it now only for the single combination needed by the 
`TableMetadataBuilder` now (Schemaless compatible with Unbound)
   
   Let me know what you think!


-- 
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