Xuanwo commented on code in PR #645:
URL: https://github.com/apache/iceberg-rust/pull/645#discussion_r1799004933
##########
crates/iceberg/src/spec/partition.rs:
##########
@@ -235,6 +261,179 @@ impl UnboundPartitionSpec {
}
}
+/// Trait for common functions between [`PartitionSpec`],
[`UnboundPartitionSpec`] and [`PreservedPartitionSpec`]
+pub trait UnboundPartitionSpecInterface<T: PartitionFieldInterface> {
Review Comment:
Hi, compared to having a trait, I personally feel that building an `enum`
for this would be better. Maybe we could have:
```rust
enum PartitionSpec {
Bound(BoundPartitionSpec),
Unbound(UnboundPartitionSpec),
}
```
Users can convert between bound and unbound versions based on their needs.
This makes it easier to find the differences between various partition specs.
I believe this can avoid some complexity around the trait system and narrow
down our API surface. What do 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: [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]
