test: add file/SQL level test for pruning parquet row group with decimal data type. #2977
Conversation
liukun4515
changed the title
liukun4515
force-pushed
the
prune_decimal_parquet_file
branch
from
8eef9f3 to
2f88811
Compare
Codecov Report
@@ Coverage Diff @@
## master #2977 +/- ##
==========================================
+ Coverage 85.75% 85.78% +0.02%
==========================================
Files 281 281
Lines 51494 51559 +65
==========================================
+ Hits 44161 44228 +67
+ Misses 7333 7331 -2
Help us with your feedback. Take ten seconds to tell us how you rate us. |
|
Can you please also add test cases to cover the decimal type with large precisions? For example decimal(18 ,0), decimal (38, 0). Maybe the UT will fail but once the getStats methods is fixed for binary array encoded decimal, the UT should pass. |
thanks for your suggestions, and will add test for larger precision of decimal which will be stored as fixed_length_byte_array in the parquet. |
|
add more test case for other precision. |
…ixed_length_byte_array in parquet
liukun4515
force-pushed
the
prune_decimal_parquet_file
branch
from
095fb62 to
0f8f7e2
Compare
| @@ -449,6 +450,154 @@ async fn prune_int32_eq_in_list_negated() { | |||
| assert_eq!(output.result_rows, 19, "{}", output.description()); | |||
| } | |||
|
|
|||
| async fn test_prune_decimal( | |||
There was a problem hiding this comment.
I'm inspired by this, I will collation other test case and make the test cases cleanly
There was a problem hiding this comment.
Can you please clarify what you mean by this comment?
I didn't quite understand if you planned to do this combination as part of this PR or if you planned to do it as a follow on PR.
In other words, is this PR ready for review, or do you plan more work?
There was a problem hiding this comment.
Sorry for the confused comments.
When I work on the decimal test, and find all the pruning test has the common logic.
I can collation them in the next pr.
async test_prune(
case_type: Scenario,
sql: &str,
expected_errors: Option<usize>,
expected_row_group_pruned: Option<usize>,
expected_results: usize) -> {}
All the test cases such as prune_f64_lt,prune_i64_lt can use test_prune to replace the repeated logic.
There was a problem hiding this comment.
follow up pr: #2986
| @@ -449,6 +450,154 @@ async fn prune_int32_eq_in_list_negated() { | |||
| assert_eq!(output.result_rows, 19, "{}", output.description()); | |||
| } | |||
|
|
|||
| async fn test_prune_decimal( | |||
There was a problem hiding this comment.
Can you please clarify what you mean by this comment?
I didn't quite understand if you planned to do this combination as part of this PR or if you planned to do it as a follow on PR.
In other words, is this PR ready for review, or do you plan more work?
|
I will merge this pr first, and collation all the code of the |
|
Benchmark runs are scheduled for baseline = 2d23860 and contender = 3d1de15. 3d1de15 is a master commit associated with this PR. Results will be available as each benchmark for each run completes. |
Which issue does this PR close?
This PR just add more test cases for pruning row group with the decimal data type in parquet file.
part of #2962
Rationale for this change
What changes are included in this PR?
Are there any user-facing changes?