blackmwk commented on code in PR #2951:
URL: https://github.com/apache/iceberg-rust/pull/2951#discussion_r3932211385
##########
crates/iceberg/src/spec/values/serde.rs:
##########
@@ -73,10 +97,10 @@ pub(crate) mod _serde {
let len = self.required.len() + self.optional.len();
let mut record = serializer.serialize_struct("", len)?;
for (k, v) in &self.required {
- record.serialize_field(Box::leak(k.clone().into_boxed_str()),
&v)?;
+ record.serialize_field(intern_field_name(k), &v)?;
Review Comment:
I don't think this is the right direction. Instead of serializing as struct,
we could serialize as a map and we could then avoid interning.
##########
crates/iceberg/src/spec/values/serde.rs:
##########
@@ -716,4 +740,61 @@ pub(crate) mod _serde {
}
}
}
+
+ #[cfg(test)]
Review Comment:
Put the tests in values's tests mod
--
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]
