fix: issue #8922 make row group test more readable #8941
Conversation
Lordworms
changed the title
There was a problem hiding this comment.
Thank you so much @Lordworms -- this is looking really good
There seems to be one difference in prune_int32_eq_large_in_list but otherwise I think this PR is ready to go.
cc @Ted-Jiang as this will likely conflict with #8930
| 0, | ||
| ) | ||
| .await; | ||
| .with_expected_errors(Some(0)) |
There was a problem hiding this comment.
This seems like the test is different -- the results are 0, 0, 1 rather than 0,1,0
Perhaps there is a difference between test_row_group_prune and test_prune_verbose
There was a problem hiding this comment.
OK, I am confused about these two functions since they look and execute the same logic. I hope to get an answer about the differences
There was a problem hiding this comment.
Actually, I just double checked and the difference appears to be that the argument order was different to the two functions 🤦
So therefore I think this change makes sense
| .with_scenario(Scenario::PeriodsInColumnNames) | ||
| .with_query( "SELECT \"name\", \"service.name\" FROM t WHERE \"service.name\" = 'frontend'") | ||
| .with_expected_errors(Some(0)) | ||
| .with_pruned_by_stats(Some(1)) |
There was a problem hiding this comment.
| .with_pruned_by_stats(Some(1)) | |
| // prune out first and last row group | |
| .with_pruned_by_stats(Some(1)) |
| .with_scenario(Scenario::PeriodsInColumnNames) | ||
| .with_query( "SELECT \"name\", \"service.name\" FROM t WHERE \"name\" != 'HTTP GET / DISPATCH'") | ||
| .with_expected_errors(Some(0)) | ||
| .with_pruned_by_stats(Some(2)) |
There was a problem hiding this comment.
| .with_pruned_by_stats(Some(2)) | |
| // prune out first and last row group | |
| .with_pruned_by_stats(Some(2)) |
| .with_scenario(Scenario::PeriodsInColumnNames) | ||
| .with_query( "SELECT \"name\", \"service.name\" FROM t WHERE \"service.name\" = 'frontend' AND \"name\" != 'HTTP GET / DISPATCH'") | ||
| .with_expected_errors(Some(0)) | ||
| .with_pruned_by_stats(Some(2)) |
There was a problem hiding this comment.
| .with_pruned_by_stats(Some(2)) | |
| // prune out first and last row group | |
| .with_pruned_by_stats(Some(2)) |
|
an unused variable that I used to debug has not been deleted, I'll fix it soon |
|
FYI @Ted-Jiang |
Which issue does this PR close?
Closes #8922.
Rationale for this change
Since the original test in datafusion/core/tests/parquet/row_group_pruning.rs is hard to read, I use a test structure mentioned by lamb in comment to refracture the test
What changes are included in this PR?
Are these changes tested?
yes
Are there any user-facing changes?