Measure the number of Parquet row groups filtered by predicate pushdown #17594
Conversation
|
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. |
mhaseeb123
added
2 - In Progress
mhaseeb123
changed the title
| // 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 |
There was a problem hiding this comment.
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. |
There was a problem hiding this comment.
Moved this piece outside in select_row_groups() to take in input_row_group_indices as input param
cpp/include/cudf/io/types.hpp
Outdated
| 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 |
There was a problem hiding this comment.
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
| 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), |
There was a problem hiding this comment.
static_cast simply reformatted this line
|
|
||
| // The following variables are currently only computed for Parquet reader | ||
| size_type num_input_row_groups{0}; //!< Total number of input row groups across all data sources | ||
| std::optional<size_type> |
There was a problem hiding this comment.
Edit: With recent updates to this PR:
num_row_groups_after_stats_filterwill bestd::nulloptifnot filter.has_value()and equal tonum_input_row_groupsor actual value otherwisenum_row_groups_after_bloom_filterwill bestd::nulloptifnot filter.has_value() or not bloom_filter_existand equal tonum_row_groups_after_stats_filteror actual value otherwise.
| // pushdown. | ||
| out_metadata.num_input_row_groups = _file_itm_data.num_input_row_groups; | ||
| // Copy the number surviving row groups from each predicate pushdown only if the filter has value. | ||
| if (_expr_conv.get_converted_expr().has_value()) { |
There was a problem hiding this comment.
Copy from _file_itm_data if filter.has_value()
| @@ -451,29 +428,55 @@ std::optional<std::vector<std::vector<size_type>>> aggregate_reader_metadata::fi | |||
| // Converts AST to StatsAST with reference to min, max columns in above `stats_table`. | |||
| stats_expression_converter const stats_expr{filter.get(), | |||
| static_cast<size_type>(output_dtypes.size())}; | |||
| auto stats_ast = stats_expr.get_stats_expr(); | |||
There was a problem hiding this comment.
Removed this stale code (already moved inside collect_filtered_row_group_indices)
|
|
||
| // Filter stats table with StatsAST expression and collect filtered row group indices | ||
| auto const filtered_row_group_indices = collect_filtered_row_group_indices( | ||
| stats_table, stats_expr.get_stats_expr(), input_row_group_indices, stream); | ||
|
|
||
| // Number of surviving row groups after applying stats filter | ||
| auto const num_stats_filtered_row_groups = |
There was a problem hiding this comment.
There isn't really a straightforward way in here to check if stats weren't available so this will be set to either total_row_groups or the filtered number of row groups.
There was a problem hiding this comment.
so if there is a filter, but no stats, we still report a num_stats_filtered_row_groups number, even though we didn't really do any stats-based filtering?
There was a problem hiding this comment.
Yeah unfortunately, not that straightforward to check the availability of stats right now!
There was a problem hiding this comment.
Created #17864 to handle this in future PR
|
|
||
| // Filter stats table with StatsAST expression and collect filtered row group indices | ||
| auto const filtered_row_group_indices = collect_filtered_row_group_indices( | ||
| stats_table, stats_expr.get_stats_expr(), input_row_group_indices, stream); | ||
|
|
||
| // Number of surviving row groups after applying stats filter | ||
| auto const num_stats_filtered_row_groups = |
There was a problem hiding this comment.
so if there is a filter, but no stats, we still report a num_stats_filtered_row_groups number, even though we didn't really do any stats-based filtering?
mhaseeb123
added
feature request
|
/merge |
mhaseeb123
added
5 - Ready to Merge
Description
Closes #17164
This PR adds a method to measure the number of remaining row groups after stats and bloom filtering during predicate pushdown.
Checklist