Stefan-Dienst commented on code in PR #2802:
URL: https://github.com/apache/iceberg-rust/pull/2802#discussion_r4048012043
##########
crates/iceberg/src/spec/partition.rs:
##########
@@ -34,10 +34,18 @@ pub(crate) const DEFAULT_PARTITION_SPEC_ID: i32 = 0;
/// Partition fields capture the transform from table data to partition values.
#[derive(Debug, Serialize, Deserialize, PartialEq, Eq, Clone, TypedBuilder)]
-#[serde(rename_all = "kebab-case")]
+#[serde(
+ try_from = "_serde_partition_field::PartitionFieldSerde",
+ into = "_serde_partition_field::PartitionFieldSerde"
+)]
pub struct PartitionField {
/// A source column id from the table’s schema
pub source_id: i32,
+ /// Source column ids when the transform takes multiple arguments (v3
multi-argument
+ /// transforms). `None` for single-argument transforms, where `source_id`
is used instead.
+ /// When set, `source_id` holds the first id so that existing consumers
keep working.
+ #[builder(default)]
+ pub source_ids: Option<Vec<i32>>,
Review Comment:
Just to clarify: I did not mean to add a version field, but have a different
struct for the different spec versions. For example something like:
```rust
#[derive(Debug, Serialize, Deserialize, PartialEq, Eq, Clone)]
#[serde(rename_all = "kebab-case")]
pub(super) struct PartitionFieldV3 {
#[serde(default, skip_serializing_if = "Option::is_none")]
source_id: Option<i32>,
#[serde(default, skip_serializing_if = "Option::is_none")]
source_ids: Option<Vec<i32>>,
field_id: i32,
name: String,
transform: Option<String>,
}
#[derive(Debug, Serialize, Deserialize, PartialEq, Eq, Clone)]
#[serde(rename_all = "kebab-case")]
pub(super) struct PartitionFieldV2 {
source_id: i32,
field_id: i32,
name: String,
transform: Option<String>
}
#[derive(Debug, Serialize, Deserialize, PartialEq, Eq, Clone)]
#[serde(rename_all = "kebab-case")]
pub(super) struct PartitionFieldV1 {
source_id: i32,
name: String,
transform: Option<String>
}
```
and then use this in the table metadata serdes like `TableMetadataV2`.
Then the in memory structure could be changed to (taking
https://github.com/apache/iceberg-rust/issues/3172 into account)
```rust
pub struct PartitionField {
source_ids: Vec<i32>,
[...]
```
and the `impl TryFrom` and `impl From` handle the different specs versions.
Does this make sense?
--
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]
