blackmwk commented on code in PR #3091:
URL: https://github.com/apache/iceberg-rust/pull/3091#discussion_r3891555457


##########
crates/iceberg/src/scan/task.rs:
##########
@@ -282,3 +246,147 @@ pub struct FileScanTaskDeleteFile {
     #[builder(default)]
     pub key_metadata: Option<Box<[u8]>>,
 }
+
+mod _serde {
+    use serde_derive::{Deserialize as DeserializeDerive, Serialize as 
SerializeDerive};
+
+    use super::*;
+    use crate::{Error, ErrorKind};
+
+    // Container-level `into` conversion is infallible. Keep a failed typed 
conversion here so
+    // serialization can return that error instead of panicking or changing 
the wire format.
+    struct PartitionSerde(Result<RawLiteral>);
+
+    impl Serialize for PartitionSerde {
+        fn serialize<S>(&self, serializer: S) -> std::result::Result<S::Ok, 
S::Error>
+        where S: serde::Serializer {
+            self.0
+                .as_ref()
+                .map_err(serde::ser::Error::custom)?
+                .serialize(serializer)
+        }
+    }
+
+    impl<'de> Deserialize<'de> for PartitionSerde {
+        fn deserialize<D>(deserializer: D) -> std::result::Result<Self, 
D::Error>
+        where D: serde::Deserializer<'de> {
+            Ok(Self(Ok(RawLiteral::deserialize(deserializer)?)))
+        }
+    }
+
+    #[derive(SerializeDerive, DeserializeDerive)]
+    pub(super) struct FileScanTaskSerde {
+        file_size_in_bytes: u64,
+        start: u64,
+        length: u64,
+        record_count: Option<u64>,
+        #[serde(skip_serializing_if = "Option::is_none")]
+        first_row_id: Option<i64>,
+        #[serde(skip_serializing_if = "Option::is_none")]
+        data_sequence_number: Option<i64>,
+        data_file_path: String,
+        data_file_format: DataFileFormat,
+        schema: SchemaRef,
+        project_field_ids: Vec<i32>,
+        #[serde(skip_serializing_if = "Option::is_none")]
+        predicate: Option<BoundPredicate>,
+        deletes: Vec<FileScanTaskDeleteFile>,
+        #[serde(default)]
+        #[serde(skip_serializing_if = "Option::is_none")]
+        partition: Option<PartitionSerde>,
+        #[serde(default)]
+        #[serde(skip_serializing_if = "Option::is_none")]
+        partition_spec: Option<Arc<PartitionSpec>>,
+        #[serde(default)]
+        #[serde(skip_serializing_if = "Option::is_none")]
+        name_mapping: Option<Arc<NameMapping>>,
+        #[serde(default)]
+        #[serde(skip_serializing_if = "Option::is_none")]
+        unified_partition_type: Option<Arc<StructType>>,
+        case_sensitive: bool,
+        #[serde(default)]
+        #[serde(skip_serializing_if = "Option::is_none")]
+        key_metadata: Option<Box<[u8]>>,
+    }
+
+    fn partition_type(schema: &Schema, partition_spec: Option<&PartitionSpec>) 
-> Result<Type> {

Review Comment:
   Removed the partition_type helper and now derive the type directly from the 
selected partition spec.



##########
crates/iceberg/src/scan/task.rs:
##########
@@ -282,3 +246,147 @@ pub struct FileScanTaskDeleteFile {
     #[builder(default)]
     pub key_metadata: Option<Box<[u8]>>,
 }
+
+mod _serde {
+    use serde_derive::{Deserialize as DeserializeDerive, Serialize as 
SerializeDerive};
+
+    use super::*;
+    use crate::{Error, ErrorKind};
+
+    // Container-level `into` conversion is infallible. Keep a failed typed 
conversion here so
+    // serialization can return that error instead of panicking or changing 
the wire format.
+    struct PartitionSerde(Result<RawLiteral>);

Review Comment:
   Removed PartitionSerde; the serde representation now stores 
Option<RawLiteral> directly.



##########
crates/iceberg/src/scan/task.rs:
##########
@@ -282,3 +246,147 @@ pub struct FileScanTaskDeleteFile {
     #[builder(default)]
     pub key_metadata: Option<Box<[u8]>>,
 }
+
+mod _serde {
+    use serde_derive::{Deserialize as DeserializeDerive, Serialize as 
SerializeDerive};
+
+    use super::*;
+    use crate::{Error, ErrorKind};
+
+    // Container-level `into` conversion is infallible. Keep a failed typed 
conversion here so
+    // serialization can return that error instead of panicking or changing 
the wire format.
+    struct PartitionSerde(Result<RawLiteral>);
+
+    impl Serialize for PartitionSerde {
+        fn serialize<S>(&self, serializer: S) -> std::result::Result<S::Ok, 
S::Error>
+        where S: serde::Serializer {
+            self.0
+                .as_ref()
+                .map_err(serde::ser::Error::custom)?
+                .serialize(serializer)
+        }
+    }
+
+    impl<'de> Deserialize<'de> for PartitionSerde {
+        fn deserialize<D>(deserializer: D) -> std::result::Result<Self, 
D::Error>
+        where D: serde::Deserializer<'de> {
+            Ok(Self(Ok(RawLiteral::deserialize(deserializer)?)))
+        }
+    }
+
+    #[derive(SerializeDerive, DeserializeDerive)]
+    pub(super) struct FileScanTaskSerde {
+        file_size_in_bytes: u64,
+        start: u64,
+        length: u64,
+        record_count: Option<u64>,
+        #[serde(skip_serializing_if = "Option::is_none")]
+        first_row_id: Option<i64>,
+        #[serde(skip_serializing_if = "Option::is_none")]
+        data_sequence_number: Option<i64>,
+        data_file_path: String,
+        data_file_format: DataFileFormat,
+        schema: SchemaRef,
+        project_field_ids: Vec<i32>,
+        #[serde(skip_serializing_if = "Option::is_none")]
+        predicate: Option<BoundPredicate>,
+        deletes: Vec<FileScanTaskDeleteFile>,
+        #[serde(default)]
+        #[serde(skip_serializing_if = "Option::is_none")]
+        partition: Option<PartitionSerde>,
+        #[serde(default)]
+        #[serde(skip_serializing_if = "Option::is_none")]
+        partition_spec: Option<Arc<PartitionSpec>>,
+        #[serde(default)]
+        #[serde(skip_serializing_if = "Option::is_none")]
+        name_mapping: Option<Arc<NameMapping>>,
+        #[serde(default)]
+        #[serde(skip_serializing_if = "Option::is_none")]
+        unified_partition_type: Option<Arc<StructType>>,
+        case_sensitive: bool,
+        #[serde(default)]
+        #[serde(skip_serializing_if = "Option::is_none")]
+        key_metadata: Option<Box<[u8]>>,
+    }
+
+    fn partition_type(schema: &Schema, partition_spec: Option<&PartitionSpec>) 
-> Result<Type> {
+        Ok(Type::Struct(match partition_spec {
+            Some(partition_spec) => partition_spec.partition_type(schema)?,
+            None => StructType::new(vec![]),
+        }))
+    }
+
+    impl From<FileScanTask> for FileScanTaskSerde {
+        fn from(value: FileScanTask) -> Self {
+            let partition = value.partition.map(|partition| {

Review Comment:
   Updated both serialization and deserialization to fall back to 
PartitionSpec::unpartition_spec() when partition_spec is absent.



-- 
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]

Reply via email to