16pierre commented on code in PR #19928:
URL: https://github.com/apache/datafusion/pull/19928#discussion_r2712174704


##########
datafusion/pruning/src/pruning_predicate.rs:
##########
@@ -3229,14 +3267,19 @@ mod tests {
     #[test]
     fn row_group_predicate_in_list_to_many_values() -> Result<()> {
         let schema = Schema::new(vec![Field::new("c1", DataType::Int32, 
false)]);
-        // test c1 in(1..21)
-        // in pruning.rs has MAX_LIST_VALUE_SIZE_REWRITE = 20, more than this 
value will be rewrite
-        // always true
-        let expr = col("c1").in_list((1..=21).map(lit).collect(), false);
+        let limit = 15i32;
+        let expr = col("c1").in_list((1..=limit).map(lit).collect(), false);
 
         let expected_expr = "true";
         let predicate_expr =
-            test_build_predicate_expression(&expr, &schema, &mut 
RequiredColumns::new());
+            test_build_predicate_expression_with_pruning_predicate_config(

Review Comment:
   `row_group_predicate_in_list_to_many_values` felt like an obvious utest to 
verify the configuration is applied, I may be missing some extra test coverage 
though; in particular, this change is missing integration tests that verify the 
end-to-end integration of the new Parquet configuration. Not sure if extra 
coverage is required, and if so where to implement it, happy to get guidance.



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

Reply via email to