Fokko commented on code in PR #309:
URL: https://github.com/apache/iceberg-rust/pull/309#discussion_r1542486911
##########
crates/iceberg/src/spec/transform.rs:
##########
@@ -398,6 +679,80 @@ mod tests {
}
}
+ #[test]
+ fn test_none_projection() -> Result<()> {
+ let name = "projected_name".to_string();
+ let preds = TestPredicates::new();
+
+ let transform = Transform::Void;
+ let result_unary = transform.project(name.clone(), &preds.unary)?;
+ assert!(result_unary.is_none());
+
+ let transform = Transform::Year;
+ let result_binary = transform.project(name.clone(), &preds.binary)?;
+ assert!(result_binary.is_none());
+
+ let transform = Transform::Month;
+ let result_set = transform.project(name.clone(), &preds.set)?;
+ assert!(result_set.is_none());
+
+ Ok(())
+ }
+
+ #[test]
+ fn test_truncate_project() -> Result<()> {
Review Comment:
Could you port all the tests from Java:
https://github.com/apache/iceberg/blob/main/api/src/test/java/org/apache/iceberg/transforms/TestTruncatesProjection.java
##########
crates/iceberg/src/spec/transform.rs:
##########
@@ -398,6 +679,80 @@ mod tests {
}
}
+ #[test]
+ fn test_none_projection() -> Result<()> {
+ let name = "projected_name".to_string();
+ let preds = TestPredicates::new();
+
+ let transform = Transform::Void;
+ let result_unary = transform.project(name.clone(), &preds.unary)?;
+ assert!(result_unary.is_none());
+
+ let transform = Transform::Year;
+ let result_binary = transform.project(name.clone(), &preds.binary)?;
+ assert!(result_binary.is_none());
+
+ let transform = Transform::Month;
+ let result_set = transform.project(name.clone(), &preds.set)?;
+ assert!(result_set.is_none());
+
+ Ok(())
+ }
+
+ #[test]
+ fn test_truncate_project() -> Result<()> {
+ let name = "projected_name".to_string();
+ let preds = TestPredicates::new();
+
+ let transform = Transform::Truncate(10);
+
+ let result_unary = transform.project(name.clone(),
&preds.unary)?.unwrap();
+ let result_binary = transform.project(name.clone(),
&preds.binary)?.unwrap();
+ let result_set = transform.project(name.clone(), &preds.set)?.unwrap();
+
+ assert_eq!(format!("{}", result_unary), "projected_name IS NULL");
+ assert_eq!(format!("{}", result_binary), "projected_name = 0");
+ assert_eq!(format!("{}", result_set), "projected_name IN (0)");
+
+ Ok(())
+ }
+
+ #[test]
+ fn test_identity_project() -> Result<()> {
+ let name = "projected_name".to_string();
+ let preds = TestPredicates::new();
+
+ let transform = Transform::Identity;
+
+ let result_unary = transform.project(name.clone(),
&preds.unary)?.unwrap();
+ let result_binary = transform.project(name.clone(),
&preds.binary)?.unwrap();
+ let result_set = transform.project(name.clone(), &preds.set)?.unwrap();
+
+ assert_eq!(format!("{}", result_unary), "projected_name IS NULL");
+ assert_eq!(format!("{}", result_binary), "projected_name = 5");
+ assert_eq!(format!("{}", result_set), "projected_name IN (5, 6)");
+
+ Ok(())
+ }
+
+ #[test]
+ fn test_bucket_project() -> Result<()> {
Review Comment:
Can you port the test suite from Java to Rust:
https://github.com/apache/iceberg/blob/main/api/src/test/java/org/apache/iceberg/transforms/TestBucketingProjection.java
##########
crates/iceberg/src/spec/transform.rs:
##########
@@ -398,6 +679,80 @@ mod tests {
}
}
+ #[test]
+ fn test_none_projection() -> Result<()> {
+ let name = "projected_name".to_string();
+ let preds = TestPredicates::new();
+
+ let transform = Transform::Void;
+ let result_unary = transform.project(name.clone(), &preds.unary)?;
+ assert!(result_unary.is_none());
+
+ let transform = Transform::Year;
+ let result_binary = transform.project(name.clone(), &preds.binary)?;
+ assert!(result_binary.is_none());
+
+ let transform = Transform::Month;
+ let result_set = transform.project(name.clone(), &preds.set)?;
+ assert!(result_set.is_none());
+
+ Ok(())
+ }
+
+ #[test]
+ fn test_truncate_project() -> Result<()> {
+ let name = "projected_name".to_string();
+ let preds = TestPredicates::new();
+
+ let transform = Transform::Truncate(10);
+
+ let result_unary = transform.project(name.clone(),
&preds.unary)?.unwrap();
+ let result_binary = transform.project(name.clone(),
&preds.binary)?.unwrap();
+ let result_set = transform.project(name.clone(), &preds.set)?.unwrap();
+
+ assert_eq!(format!("{}", result_unary), "projected_name IS NULL");
+ assert_eq!(format!("{}", result_binary), "projected_name = 0");
+ assert_eq!(format!("{}", result_set), "projected_name IN (0)");
+
+ Ok(())
+ }
+
+ #[test]
+ fn test_identity_project() -> Result<()> {
+ let name = "projected_name".to_string();
+ let preds = TestPredicates::new();
+
+ let transform = Transform::Identity;
+
+ let result_unary = transform.project(name.clone(),
&preds.unary)?.unwrap();
+ let result_binary = transform.project(name.clone(),
&preds.binary)?.unwrap();
+ let result_set = transform.project(name.clone(), &preds.set)?.unwrap();
+
+ assert_eq!(format!("{}", result_unary), "projected_name IS NULL");
+ assert_eq!(format!("{}", result_binary), "projected_name = 5");
+ assert_eq!(format!("{}", result_set), "projected_name IN (5, 6)");
+
+ Ok(())
+ }
+
+ #[test]
+ fn test_bucket_project() -> Result<()> {
+ let name = "projected_name".to_string();
+ let preds = TestPredicates::new();
+
+ let transform = Transform::Bucket(8);
+
+ let result_unary = transform.project(name.clone(),
&preds.unary)?.unwrap();
+ let result_binary = transform.project(name.clone(),
&preds.binary)?.unwrap();
+ let result_set = transform.project(name.clone(), &preds.set)?.unwrap();
+
+ assert_eq!(format!("{}", result_unary), "projected_name IS NULL");
+ assert_eq!(format!("{}", result_binary), "projected_name = 7");
+ assert_eq!(format!("{}", result_set), "projected_name IN (1, 7)");
+
+ Ok(())
+ }
+
Review Comment:
I think the most important tests are missing around the dates projection:
https://github.com/apache/iceberg/blob/main/api/src/test/java/org/apache/iceberg/transforms/TestDatesProjection.java
##########
crates/iceberg/src/spec/transform.rs:
##########
@@ -398,6 +679,80 @@ mod tests {
}
}
+ #[test]
+ fn test_none_projection() -> Result<()> {
+ let name = "projected_name".to_string();
+ let preds = TestPredicates::new();
+
+ let transform = Transform::Void;
+ let result_unary = transform.project(name.clone(), &preds.unary)?;
+ assert!(result_unary.is_none());
+
+ let transform = Transform::Year;
+ let result_binary = transform.project(name.clone(), &preds.binary)?;
+ assert!(result_binary.is_none());
+
+ let transform = Transform::Month;
+ let result_set = transform.project(name.clone(), &preds.set)?;
+ assert!(result_set.is_none());
+
+ Ok(())
+ }
+
+ #[test]
+ fn test_truncate_project() -> Result<()> {
+ let name = "projected_name".to_string();
+ let preds = TestPredicates::new();
+
+ let transform = Transform::Truncate(10);
+
+ let result_unary = transform.project(name.clone(),
&preds.unary)?.unwrap();
+ let result_binary = transform.project(name.clone(),
&preds.binary)?.unwrap();
+ let result_set = transform.project(name.clone(), &preds.set)?.unwrap();
+
+ assert_eq!(format!("{}", result_unary), "projected_name IS NULL");
+ assert_eq!(format!("{}", result_binary), "projected_name = 0");
+ assert_eq!(format!("{}", result_set), "projected_name IN (0)");
Review Comment:
Example can be found here:
https://github.com/apache/iceberg/blob/81b62c78e0c230516090becda7d6040ee03e6a91/api/src/test/java/org/apache/iceberg/transforms/TestTruncatesProjection.java#L189-L190
It would be good to port the other tests of Java as well.
##########
crates/iceberg/src/spec/transform.rs:
##########
@@ -398,6 +679,80 @@ mod tests {
}
}
+ #[test]
+ fn test_none_projection() -> Result<()> {
+ let name = "projected_name".to_string();
+ let preds = TestPredicates::new();
+
+ let transform = Transform::Void;
+ let result_unary = transform.project(name.clone(), &preds.unary)?;
+ assert!(result_unary.is_none());
+
+ let transform = Transform::Year;
+ let result_binary = transform.project(name.clone(), &preds.binary)?;
+ assert!(result_binary.is_none());
+
+ let transform = Transform::Month;
+ let result_set = transform.project(name.clone(), &preds.set)?;
+ assert!(result_set.is_none());
+
+ Ok(())
+ }
+
+ #[test]
+ fn test_truncate_project() -> Result<()> {
+ let name = "projected_name".to_string();
+ let preds = TestPredicates::new();
+
+ let transform = Transform::Truncate(10);
+
+ let result_unary = transform.project(name.clone(),
&preds.unary)?.unwrap();
+ let result_binary = transform.project(name.clone(),
&preds.binary)?.unwrap();
+ let result_set = transform.project(name.clone(), &preds.set)?.unwrap();
+
+ assert_eq!(format!("{}", result_unary), "projected_name IS NULL");
+ assert_eq!(format!("{}", result_binary), "projected_name = 0");
+ assert_eq!(format!("{}", result_set), "projected_name IN (0)");
Review Comment:
I think it is better to have more than one element in the set. In Python and
Java a `IN (0)` is being rewritten to `= 0` before evaluated.
--
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: issues-unsubscr...@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscr...@iceberg.apache.org
For additional commands, e-mail: issues-h...@iceberg.apache.org
