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: [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]
