alamb commented on code in PR #22643: URL: https://github.com/apache/datafusion/pull/22643#discussion_r3335057827
########## datafusion/sqllogictest/test_files/push_down_filter_regression.slt: ########## @@ -275,17 +275,25 @@ drop table agg_dyn_e2e; statement ok set datafusion.execution.target_partitions = 2; -# --- single-column fixture ([5, 1, 3, 8]) split across 2 files --- +# --- single-column fixture ([1, 8, 1, 8]) split across 2 files --- +# +# Every file shares the same per-file min (1) and max (8). This makes the Review Comment: 👍 ########## datafusion/sqllogictest/test_files/push_down_filter_regression.slt: ########## @@ -296,10 +304,11 @@ LOCATION 'test_files/scratch/push_down_filter_regression/agg_dyn_single/'; # Use `analyze_level = summary` + `analyze_categories = 'none'` so metrics # render empty; we only care that the `predicate=DynamicFilter [ ... ]` text -# matches. Pruning metrics here are subject to a parallel-execution race +# matches. The pruning *counts* are still subject to a parallel-execution race Review Comment: My concern with this approach is that it is changing the meaning of the test However, it looks like we have had issues with this test before that @mbutrovich saw - https://github.com/apache/datafusion/pull/21657 So given that I agree the actual dynamic filter content is subject to a race condition this is best I can imagine us doing at this point -- 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]
