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


##########
crates/iceberg/src/spec/transform.rs:
##########
@@ -137,24 +142,138 @@ pub enum Transform {
 
 impl Transform {
     /// Returns a human-readable String representation of a transformed value.
+    ///
+    /// The temporal transforms store an ordinal count since the Unix epoch, 
and
+    /// this method renders that count as a date so that partition paths and
+    /// snapshot summary keys match the Java reference implementation:
+    ///
+    /// | Transform | Format          | Example         |
+    /// |-----------|-----------------|-----------------|
+    /// | `Year`    | `yyyy`          | `2017`          |
+    /// | `Month`   | `yyyy-MM`       | `2017-06`       |
+    /// | `Day`     | `yyyy-MM-dd`    | `2017-06-15`    |
+    /// | `Hour`    | `yyyy-MM-dd-HH` | `2017-06-15-16` |
+    ///
+    /// `Void` renders as `null`, as does an absent value for any transform.
+    ///
+    /// # Example
+    ///
+    /// ```
+    /// use iceberg::spec::{Literal, PrimitiveType, Transform, Type};
+    ///
+    /// let int = Type::Primitive(PrimitiveType::Int);
+    /// let date = Type::Primitive(PrimitiveType::Date);
+    ///
+    /// // A stored value carries no logical type of its own. For transforms 
that do
+    /// // not format it themselves, the declared field type decides how it 
renders.
+    /// let stored = Literal::int(17332);
+    /// assert_eq!(
+    ///     Transform::Identity.to_human_string(&int, Some(&stored)),
+    ///     "17332"
+    /// );
+    /// assert_eq!(
+    ///     Transform::Identity.to_human_string(&date, Some(&stored)),
+    ///     "2017-06-15"
+    /// );
+    ///
+    /// // The temporal transforms format their own ordinal and ignore the 
declared
+    /// // type. All four ordinals below are the same instant, 
2017-06-15T16:00:00Z,
+    /// // counted at four granularities.
+    /// assert_eq!(
+    ///     Transform::Year.to_human_string(&int, Some(&Literal::int(47))),
+    ///     "2017"
+    /// );
+    /// assert_eq!(
+    ///     Transform::Month.to_human_string(&int, Some(&Literal::int(569))),
+    ///     "2017-06"
+    /// );
+    /// assert_eq!(
+    ///     Transform::Day.to_human_string(&int, Some(&Literal::int(17332))),
+    ///     "2017-06-15"
+    /// );
+    /// assert_eq!(
+    ///     Transform::Hour.to_human_string(&int, Some(&Literal::int(415984))),
+    ///     "2017-06-15-16"
+    /// );
+    ///
+    /// // `Void` and an absent value render as `null`.
+    /// assert_eq!(
+    ///     Transform::Void.to_human_string(&int, Some(&Literal::int(47))),
+    ///     "null"
+    /// );
+    /// assert_eq!(Transform::Year.to_human_string(&int, None), "null");
+    /// ```
     pub fn to_human_string(&self, field_type: &Type, value: Option<&Literal>) 
-> String {
-        let Some(value) = value else {
+        let Some(value) = value.and_then(Literal::as_primitive_literal) else {

Review Comment:
   Thanks for explaination, I think you are right.



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