iceberg-rust
iceberg-rust copied to clipboard
refactor: make `PartitionSpec` more safe
This discussion is a follow up of this comment, in summary, I'm thinking about make PartitionSpec safe with following changes:
- Making all fields private to this struct.
- Add a
schemafield toPartitionSpec.
This will introduce several changes:
PartitionSpeccould only be built using builder, which has checks for serveral parts to ensure it's correctPartitionSpec's partition type could be infered by itself- We may need to change the deserialization of
TableMetadatato use builder to build it.
cc @c-thiel @Xuanwo
@liurenjie1024 we just need to be very careful with old PartitionSpecs: They might not be valid anymore for the current schema - yet, we still want to keep them.
In these cases Java has schema field pointing to the current schema, even if fields might not be present. Java achieves this by using build_unchecked.
In think it would be much cleaner to use UnboundPartitionSpecs for these cases.
We could change TableMetadata to
pub struct TableMetadata {
...
pub(crate) partition_specs: HashMap<i32, UnboundPartitionSpecRef>, // Changed to unbound
pub(crate) default_spec: PartitionSpecRef, // This is a new field bound to the current schema.
// Remove: default_spec_id - get it from default_spec.id
}