test: Add stats based parquet file filter test#16709
Closed
PingLiuPing wants to merge 1 commit intofacebookincubator:mainfrom
Closed
test: Add stats based parquet file filter test#16709PingLiuPing wants to merge 1 commit intofacebookincubator:mainfrom
PingLiuPing wants to merge 1 commit intofacebookincubator:mainfrom
Conversation
meta-cla
bot
added
the
CLA Signed
✅ Deploy Preview for meta-velox canceled.
|
Collaborator
Author
|
Without #16700, test case reports following error: |
mbasmanova
reviewed
Mar 11, 2026
| // Filter c0 > 1000: neither file has values > 1000, both files skipped. | ||
| { | ||
| auto plan = | ||
| PlanBuilder(pool_.get()).tableScan(schema, {"c0 > 1000"}).planNode(); |
Contributor
There was a problem hiding this comment.
It seems all these tests run through the same logic and can be deduped using 3 parameters:
- filter
- expected skippedSplits
- expected processedSplits
Collaborator
Author
There was a problem hiding this comment.
@mbasmanova Thank you. Simplified the test by adding a lambda.
PingLiuPing
force-pushed
the
lp_add_parquet_test
branch
from
3853840 to
11a829a
Compare
mbasmanova
reviewed
Mar 11, 2026
|
|
||
| // File 1: integers [0, 99], doubles [0.0, 99.0], strings ["a".."d"]. | ||
| const vector_size_t numRows = 100; | ||
| auto file1 = TempFilePath::create()->getPath(); |
Contributor
There was a problem hiding this comment.
a1,a2,...naming is anti-pattern
{
auto filePath =...
auto data = ...
writeToParquetFile(filePath, {data}, options);
}
Collaborator
Author
There was a problem hiding this comment.
Thanks, file1 and file2 will be used to create the splits later.
Refined the name.
Contributor
There was a problem hiding this comment.
Since these always used together, a better pattern would be:
std::vector filePaths;
std::vector dataVectors;
{
filePaths.push_back(...);
data.push_back(...);
writeToParquetFile(filePaths.back(), {data.back()}, options);
}
mbasmanova
reviewed
Mar 11, 2026
| { | ||
| makeFlatVector<int64_t>(numRows, [](auto row) { return row + 200; }), | ||
| makeFlatVector<double>( | ||
| numRows, [](auto row) { return static_cast<double>(row + 200); }), |
Contributor
There was a problem hiding this comment.
I think you can drop static_cast<double>
mbasmanova
reviewed
Mar 11, 2026
| }); | ||
| writeToParquetFile(file2, {vector2}, options); | ||
|
|
||
| auto schema = asRowType(vector1->type()); |
Contributor
There was a problem hiding this comment.
use RowVector::rowType()
mbasmanova
reviewed
Mar 11, 2026
| auto testFileSkipping = [&](const std::string& filter, | ||
| int32_t expectedSkipped, | ||
| int32_t expectedProcessed) { | ||
| auto plan = PlanBuilder(pool_.get()).tableScan(schema, {filter}).planNode(); |
Contributor
There was a problem hiding this comment.
add SCOPED_TRACE(filter)
PingLiuPing
force-pushed
the
lp_add_parquet_test
branch
2 times, most recently
from
5e51ade to
ec7ddd2
Compare
PingLiuPing
force-pushed
the
lp_add_parquet_test
branch
from
ec7ddd2 to
17edf92
Compare
mbasmanova
approved these changes
Mar 11, 2026
mbasmanova
added
the
ready-to-merge
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
PR #16700 added support for Parquet file-level column statistics via
ParquetReader::columnStatistics(). This PR adds an end-to-end test thatverifies entire Parquet files are pruned during table scan when file-level column statistics allow the filter to eliminate all data in a file.