blackmwk commented on code in PR #2802:
URL: https://github.com/apache/iceberg-rust/pull/2802#discussion_r4045719899
##########
crates/iceberg/src/spec/partition.rs:
##########
@@ -54,6 +62,57 @@ impl PartitionField {
}
}
+mod _serde_partition_field {
Review Comment:
```suggestion
mod _serde {
```
Following others convention.
##########
crates/iceberg/src/spec/partition.rs:
##########
@@ -264,21 +324,80 @@ impl PartitionKey {
pub type UnboundPartitionSpecRef = Arc<UnboundPartitionSpec>;
/// Unbound partition field can be built without a schema and later bound to a
schema.
#[derive(Debug, Serialize, Deserialize, PartialEq, Eq, Clone, TypedBuilder)]
-#[serde(rename_all = "kebab-case")]
+#[serde(
+ try_from = "_serde_unbound_partition_field::UnboundPartitionFieldSerde",
+ into = "_serde_unbound_partition_field::UnboundPartitionFieldSerde"
+)]
pub struct UnboundPartitionField {
Review Comment:
Simiarly for `PartitionField`
##########
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 {
Review Comment:
I prefer to do what's mentioned in
https://github.com/apache/iceberg-rust/issues/3172 first.
##########
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:
I don't think we should add such a filed, for in memory data structures, we
should just use `source_ids: Vec<i32>`
##########
crates/iceberg/src/spec/partition.rs:
##########
@@ -264,21 +324,80 @@ impl PartitionKey {
pub type UnboundPartitionSpecRef = Arc<UnboundPartitionSpec>;
/// Unbound partition field can be built without a schema and later bound to a
schema.
#[derive(Debug, Serialize, Deserialize, PartialEq, Eq, Clone, TypedBuilder)]
-#[serde(rename_all = "kebab-case")]
+#[serde(
+ try_from = "_serde_unbound_partition_field::UnboundPartitionFieldSerde",
+ into = "_serde_unbound_partition_field::UnboundPartitionFieldSerde"
+)]
pub struct UnboundPartitionField {
/// 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>>,
/// A partition field id that is used to identify a partition field and is
unique within a partition spec.
/// In v2 table metadata, it is unique across all partition specs.
#[builder(default, setter(strip_option(fallback = field_id_opt)))]
- #[serde(skip_serializing_if = "Option::is_none")]
pub field_id: Option<i32>,
/// A partition name.
pub name: String,
/// A transform that is applied to the source column to produce a
partition value.
pub transform: Transform,
}
+mod _serde_unbound_partition_field {
Review Comment:
Merge with `_serde`
--
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]
