dannycjones commented on code in PR #3022:
URL: https://github.com/apache/iceberg-rust/pull/3022#discussion_r3852359680
##########
crates/iceberg/src/spec/transform.rs:
##########
@@ -1119,4 +1192,81 @@ mod tests {
check_boundary(PredicateOperator::GreaterThanOrEq, datum.clone(),
datum);
}
}
+
+ #[test]
+ fn test_human_year() {
+ assert_eq!(Transform::human_year(-1970), "0000");
+ assert_eq!(Transform::human_year(-1), "1969");
+ assert_eq!(Transform::human_year(0), "1970");
+ assert_eq!(Transform::human_year(47), "2017");
+ }
+
+ #[test]
+ fn test_human_month() {
+ assert_eq!(Transform::human_month(-1970 * 12), "0000-01");
+ assert_eq!(Transform::human_month(-13), "1968-12");
+ assert_eq!(Transform::human_month(-12), "1969-01");
+ assert_eq!(Transform::human_month(-1), "1969-12");
+ assert_eq!(Transform::human_month(0), "1970-01");
+ assert_eq!(Transform::human_month(11), "1970-12");
+ assert_eq!(Transform::human_month(12), "1971-01");
+ assert_eq!(Transform::human_month(569), "2017-06");
+ }
+
+ #[test]
+ fn test_human_hour() {
+ assert_eq!(Transform::human_hour(-24), "1969-12-31-00");
+ assert_eq!(Transform::human_hour(-1), "1969-12-31-23");
+ assert_eq!(Transform::human_hour(0), "1970-01-01-00");
+ assert_eq!(Transform::human_hour(23), "1970-01-01-23");
+ assert_eq!(Transform::human_hour(24), "1970-01-02-00");
+ assert_eq!(Transform::human_hour(1000), "1970-02-11-16");
+ }
+
+ #[test]
+ fn test_to_human_string_dispatches_on_transform() {
+ let int = Type::Primitive(PrimitiveType::Int);
+ let date = Type::Primitive(PrimitiveType::Date);
+
+ 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::Hour.to_human_string(&int, Some(&Literal::int(1000))),
+ "1970-02-11-16"
+ );
+
+ // Day needs no dispatch arm: its result type is `date`, so the datum
path
+ // already renders a calendar date. Asserted so that a later reader
knows
+ // Day was considered rather than overlooked.
+ assert_eq!(
+ Transform::Day.to_human_string(&date, Some(&Literal::int(0))),
+ "1970-01-01"
+ );
Review Comment:
Is there any risk that an `int` gets passed in here for a `Day` transform?
Other implementations of Iceberg (including Rust previously) have used `int`
to represent day rather than date.
I think my primary worry is for anyone who is running something
copy-on-write which could take an int-encoded day transform and write it back
out as int-encoded rather than date-encoded. The impact would be small though
IMO as this is not what Iceberg filtering is based on.
##########
crates/iceberg/src/spec/transform.rs:
##########
@@ -1119,4 +1192,81 @@ mod tests {
check_boundary(PredicateOperator::GreaterThanOrEq, datum.clone(),
datum);
}
}
+
+ #[test]
+ fn test_human_year() {
Review Comment:
I think the body of `test_to_human_string_dispatches_on_transform` would
make a good doctest if cleaned up a bit. Basically, something that shows
clearly how we transform a bunch of types and their transforms.
--
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]
