Fokko commented on code in PR #645:
URL: https://github.com/apache/iceberg-rust/pull/645#discussion_r1836605349
##########
crates/iceberg/src/spec/table_metadata.rs:
##########
@@ -222,21 +222,26 @@ impl TableMetadata {
/// Returns all partition specs.
#[inline]
- pub fn partition_specs_iter(&self) -> impl Iterator<Item =
&PartitionSpecRef> {
+ pub fn partition_specs_iter(&self) -> impl Iterator<Item =
&SchemalessPartitionSpecRef> {
self.partition_specs.values()
}
/// Lookup partition spec by id.
#[inline]
- pub fn partition_spec_by_id(&self, spec_id: i32) ->
Option<&PartitionSpecRef> {
+ pub fn partition_spec_by_id(&self, spec_id: i32) ->
Option<&SchemalessPartitionSpecRef> {
self.partition_specs.get(&spec_id)
}
Review Comment:
I'm not convinced that we need another type, such as
`SchemalessPartitionSpec`. Why shouldn't we return an `UnboundPartitionSpec`
instead? Looking at the signatures:
```rust
pub struct UnboundPartitionSpec {
/// Identifier for PartitionSpec
pub(crate) spec_id: Option<i32>,
/// Details of the partition spec
pub(crate) fields: Vec<UnboundPartitionField>,
}
```
```rust
pub struct SchemalessPartitionSpec {
/// Identifier for PartitionSpec
spec_id: i32,
/// Details of the partition spec
fields: Vec<PartitionField>,
}
```
And the `PartitionField` and `UnboundPartitionField` are basically the same.
> Introducing SchemalessPartitionSpec might be our way to avoid
https://github.com/apache/iceberg/issues/4563.
If we cannot find the field anymore, the best thing to do is to include the
file in the query plan.
--
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
