alamb commented on code in PR #10693:
URL: https://github.com/apache/datafusion/pull/10693#discussion_r1618624379
##########
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:
I recommend adding some other tests:
1. another negative test like `a < 5` to cover the fact that only `=`
predicates are allowed
2. A test with the literal and constant swapped (`foo = a`)
##########
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:
You can write this kind of expression more simply like
`col("a").eq(lit("foo")).and(col("b").eq(lit("bar")))` though maybe what you
have is more readable
--
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
