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

Reply via email to