houqp commented on code in PR #10693: URL: https://github.com/apache/datafusion/pull/10693#discussion_r1620256512
########## datafusion/core/src/datasource/listing/helpers.rs: ########## @@ -675,4 +764,127 @@ mod tests { // this helper function assert!(expr_applicable_for_cols(&[], &lit(true))); } + + #[test] + fn test_evaluate_partition_prefix() { + let partitions = &[ + ("a".to_string(), DataType::Utf8), + ("b".to_string(), DataType::Int16), + ("c".to_string(), DataType::Boolean), + ]; + + assert_eq!( + evaluate_partition_prefix(partitions, &[Expr::eq(col("a"), lit("foo"))],), + Some(Path::from("a=foo")), + ); + + assert_eq!( + evaluate_partition_prefix( + partitions, + &[Expr::and( + Expr::eq(col("a"), lit("foo")), + Expr::eq(col("b"), lit("bar")), + )], Review Comment: the eq helper improves readability, i applied that to all tests. i do agree with you that the `.and` helper makes the expression less readable. ########## datafusion/core/src/datasource/listing/helpers.rs: ########## @@ -675,4 +764,127 @@ mod tests { // this helper function assert!(expr_applicable_for_cols(&[], &lit(true))); } + + #[test] + fn test_evaluate_partition_prefix() { + let partitions = &[ + ("a".to_string(), DataType::Utf8), + ("b".to_string(), DataType::Int16), + ("c".to_string(), DataType::Boolean), + ]; + + assert_eq!( + evaluate_partition_prefix(partitions, &[Expr::eq(col("a"), lit("foo"))],), + Some(Path::from("a=foo")), + ); + + assert_eq!( + evaluate_partition_prefix( + partitions, + &[Expr::and( + Expr::eq(col("a"), lit("foo")), + Expr::eq(col("b"), lit("bar")), + )], + ), + Some(Path::from("a=foo/b=bar")), + ); + + assert_eq!( + evaluate_partition_prefix( + partitions, + // list of filters should be evaluated as AND + &[ + Expr::eq(col("a"), lit("foo")), + Expr::eq(col("b"), lit("bar")), + ], + ), + Some(Path::from("a=foo/b=bar")), + ); + + assert_eq!( + evaluate_partition_prefix( + partitions, + &[Expr::and( + Expr::eq(col("a"), lit("foo")), + Expr::and( + Expr::eq(col("b"), lit("1")), + Expr::eq(col("c"), lit("true")), + ), + )], + ), + Some(Path::from("a=foo/b=1/c=true")), + ); + + // no prefix when filter is empty + assert_eq!(evaluate_partition_prefix(partitions, &[]), None); + + // b=foo results in no prefix because a is not restricted Review Comment: done, good suggestions. -- 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: github-unsubscr...@datafusion.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org --------------------------------------------------------------------- To unsubscribe, e-mail: github-unsubscr...@datafusion.apache.org For additional commands, e-mail: github-h...@datafusion.apache.org