Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Measure the number of Parquet row groups filtered by predicate pushdown #17594

Open
wants to merge 15 commits into
base: branch-25.02
Choose a base branch
from

Conversation

mhaseeb123
Copy link
Member

@mhaseeb123 mhaseeb123 commented Dec 13, 2024

Description

Related to #17164

This PR adds a method to measure the number of row groups filtered using stats and bloom filters during predicate pushdown.

Checklist

  • I am familiar with the Contributing Guidelines.
  • New or existing tests cover these changes.
  • The documentation is up to date with these changes.

@mhaseeb123 mhaseeb123 self-assigned this Dec 13, 2024
Copy link

copy-pr-bot bot commented Dec 13, 2024

Auto-sync is disabled for draft pull requests in this repository. Workflows must be run manually.

Contributors can view more details about this message here.

@github-actions github-actions bot added the libcudf Affects libcudf (C++/CUDA) code. label Dec 13, 2024
@mhaseeb123 mhaseeb123 added 2 - In Progress Currently a work in progress 5 - DO NOT MERGE Hold off on merging; see PR for details improvement Improvement / enhancement to an existing function labels Dec 13, 2024
@mhaseeb123 mhaseeb123 added cuIO cuIO issue non-breaking Non-breaking change labels Dec 13, 2024
@mhaseeb123 mhaseeb123 removed the 5 - DO NOT MERGE Hold off on merging; see PR for details label Jan 15, 2025
@mhaseeb123 mhaseeb123 changed the title 🚧 Add a method to measure the number of Parquet row groups filtered by predicate pushdown Measure the number of Parquet row groups filtered by predicate pushdown Jan 16, 2025
// if filter is not empty, then gather row groups to read after predicate pushdown
if (filter.has_value()) {
filtered_row_group_indices = filter_row_groups(
sources, row_group_indices, output_dtypes, output_column_schemas, filter.value(), stream);
// Span of input row group indices for predicate pushdown
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Moved this piece of code out of filter_row_groups() to make its signature consistent with apply_bloom_filters()

host_span<data_type const> output_dtypes,
host_span<int const> output_column_schemas,
std::reference_wrapper<ast::expression const> filter,
rmm::cuda_stream_view stream) const
{
auto mr = cudf::get_current_device_resource_ref();
// Create row group indices.
Copy link
Member Author

@mhaseeb123 mhaseeb123 Jan 16, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Moved this piece outside in select_row_groups() to take in input_row_group_indices as input param

std::map<std::string, std::string> user_data; //!< Format-dependent metadata of the first input
//!< file as key-values pairs (deprecated)
std::vector<std::unordered_map<std::string, std::string>>
per_file_user_data; //!< Per file format-dependent metadata as key-values pairs

// The following variables are currently only computed for Parquet reader
size_type num_input_row_groups; //!< Number of input row groups across all data sources
Copy link
Member Author

@mhaseeb123 mhaseeb123 Jan 16, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Keeping all variables tracking number of row groups as size_type for consistency across the entire Parquet stack. Happy to change it to size_t all across in a separate PR if needed. @vuule @nvdbaranec

@mhaseeb123 mhaseeb123 marked this pull request as ready for review January 16, 2025 04:08
@mhaseeb123 mhaseeb123 requested a review from a team as a code owner January 16, 2025 04:08
@mhaseeb123 mhaseeb123 requested review from bdice, ttnghia and vuule January 16, 2025 04:08
bloom_filter_spans, parquet_types, total_row_groups, equality_col_schemas.size()};
bloom_filter_caster const bloom_filter_col{bloom_filter_spans,
parquet_types,
static_cast<size_t>(total_row_groups),
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

static_cast simply reformatted this line

@mhaseeb123 mhaseeb123 added 3 - Ready for Review Ready for review by team 4 - Needs Review Waiting for reviewer to review or respond and removed 2 - In Progress Currently a work in progress 3 - Ready for Review Ready for review by team labels Jan 16, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
4 - Needs Review Waiting for reviewer to review or respond cuIO cuIO issue improvement Improvement / enhancement to an existing function libcudf Affects libcudf (C++/CUDA) code. non-breaking Non-breaking change
Projects
Status: Burndown
Development

Successfully merging this pull request may close these issues.

1 participant