-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Hybrid scan constructor that moves in materialized footers #23795
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
base: main
Are you sure you want to change the base?
Changes from all commits
0a76858
cd9563f
7167cc4
9ed5801
1af5913
0d06e6f
12fbd21
3ce9db0
1e9df79
75d7141
b0e8d47
8da4ba9
3594c05
43f9a75
265610c
dc9b533
8dbdfb3
80dde76
5347f61
64a4137
46588d3
5cc15d6
afb797d
ec01a02
7a78a0c
afa5b57
e8b384f
1851633
dff3755
2386313
f0bfb56
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -18,7 +18,7 @@ hybrid_scan_metadata::hybrid_scan_metadata(cudf::host_span<uint8_t const> footer | |
| : _metadata{std::make_shared<detail::aggregate_reader_metadata>( | ||
| std::vector<cudf::host_span<uint8_t const>>{footer_bytes}, | ||
| options.is_enabled_use_arrow_schema(), | ||
| options.get_column_names().has_value() and options.is_enabled_allow_mismatched_pq_schemas())} | ||
| options.is_enabled_allow_mismatched_pq_schemas())} | ||
| { | ||
| } | ||
|
|
||
|
|
@@ -27,7 +27,7 @@ hybrid_scan_metadata::hybrid_scan_metadata(FileMetaData const& parquet_metadata, | |
| : _metadata{std::make_shared<detail::aggregate_reader_metadata>( | ||
| std::vector<FileMetaData>{parquet_metadata}, | ||
| options.is_enabled_use_arrow_schema(), | ||
| options.get_column_names().has_value() and options.is_enabled_allow_mismatched_pq_schemas())} | ||
| options.is_enabled_allow_mismatched_pq_schemas())} | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Same |
||
| { | ||
| } | ||
|
|
||
|
|
||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Deleting the hybrid Is this an intended change in behavior?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I pushed some commits yesterday. TLDR; both readers now only propagate REQUIRED->OPTIONAL in |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -36,22 +36,6 @@ using text::byte_range_info; | |
|
|
||
| namespace { | ||
|
|
||
| // Construct a vector of FileMetaData from the input footer bytes | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Possible regression point. Use the common parallel materializer from the base class. |
||
| [[nodiscard]] std::vector<FileMetaData> parquet_metadatas_from_footer_bytes( | ||
| cudf::host_span<cudf::host_span<uint8_t const> const> footer_bytes) | ||
| { | ||
| std::vector<FileMetaData> parquet_metadatas; | ||
| parquet_metadatas.reserve(footer_bytes.size()); | ||
| std::transform(footer_bytes.begin(), | ||
| footer_bytes.end(), | ||
| std::back_inserter(parquet_metadatas), | ||
| [](auto const& footer_bytes) { | ||
| metadata parsed_metadata{footer_bytes}; | ||
| return FileMetaData{std::move(parsed_metadata)}; | ||
| }); | ||
| return parquet_metadatas; | ||
| } | ||
|
|
||
| // Construct a vector of all row group indices from the input vectors | ||
| [[nodiscard]] auto all_row_group_indices( | ||
| std::span<std::vector<cudf::size_type> const> row_group_indices) | ||
|
|
@@ -132,25 +116,31 @@ aggregate_reader_metadata::aggregate_reader_metadata( | |
| cudf::host_span<cudf::host_span<uint8_t const> const> footer_bytes, | ||
| bool use_arrow_schema, | ||
| bool has_cols_from_mismatched_srcs) | ||
| : aggregate_reader_metadata_base(parquet_metadatas_from_footer_bytes(footer_bytes), | ||
| use_arrow_schema, | ||
| has_cols_from_mismatched_srcs) | ||
| : aggregate_reader_metadata( | ||
| parquet::detail::parallel_construct_metadatas( | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Reuses the base class's parallel footer(s) reader. |
||
| footer_bytes, [](auto const& bytes) { return FileMetaData{metadata{bytes}}; }), | ||
| use_arrow_schema, | ||
| has_cols_from_mismatched_srcs) | ||
| { | ||
| CUDF_EXPECTS( | ||
| not footer_bytes.empty(), "At least one source must be provided", std::invalid_argument); | ||
| } | ||
|
|
||
| aggregate_reader_metadata::aggregate_reader_metadata( | ||
| cudf::host_span<FileMetaData const> parquet_metadatas, | ||
| bool use_arrow_schema, | ||
| bool has_cols_from_mismatched_srcs) | ||
| : aggregate_reader_metadata_base( | ||
| : aggregate_reader_metadata( | ||
| std::vector<FileMetaData>{parquet_metadatas.begin(), parquet_metadatas.end()}, | ||
| use_arrow_schema, | ||
| has_cols_from_mismatched_srcs) | ||
| { | ||
| CUDF_EXPECTS( | ||
| not parquet_metadatas.empty(), "At least one source must be provided", std::invalid_argument); | ||
| } | ||
|
|
||
| aggregate_reader_metadata::aggregate_reader_metadata(std::vector<FileMetaData>&& parquet_metadatas, | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The only constructor that calls into base. Others just call this one |
||
| bool use_arrow_schema, | ||
| bool has_cols_from_mismatched_srcs) | ||
| : aggregate_reader_metadata_base( | ||
| std::move(parquet_metadatas), use_arrow_schema, has_cols_from_mismatched_srcs) | ||
| { | ||
| } | ||
|
|
||
| std::vector<text::byte_range_info> aggregate_reader_metadata::page_index_byte_ranges() const | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -413,19 +413,6 @@ class reader_impl { | |
| _file_itm_data._current_input_pass < _file_itm_data.num_passes(); | ||
| } | ||
|
|
||
| /** | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. No need to be protected. Move to private section |
||
| * @brief Check if the user has specified columns from mismatched sources | ||
| * | ||
| * @param options Reader options | ||
| * @return True if the user has specified columns from mismatched sources | ||
| */ | ||
| [[nodiscard]] bool has_cols_from_mismatched_sources(parquet_reader_options const& options) const | ||
| { | ||
| return (options.get_column_names().has_value() or | ||
| options.get_column_field_ids().has_value()) and | ||
| options.is_enabled_allow_mismatched_pq_schemas(); | ||
| } | ||
|
|
||
| /** | ||
| * @brief Effective `ignore_missing_columns` policy for column selection | ||
| * | ||
|
|
@@ -440,6 +427,20 @@ class reader_impl { | |
| not(has_cols_from_mismatched_sources(options) and _metadata->get_num_sources() > 1); | ||
| } | ||
|
|
||
| private: | ||
| /** | ||
| * @brief Check if the user has specified columns from mismatched sources | ||
| * | ||
| * @param options Reader options | ||
| * @return True if the user has specified columns from mismatched sources | ||
| */ | ||
| [[nodiscard]] bool has_cols_from_mismatched_sources(parquet_reader_options const& options) const | ||
| { | ||
| return (options.get_column_names().has_value() or | ||
| options.get_column_field_ids().has_value()) and | ||
| options.is_enabled_allow_mismatched_pq_schemas(); | ||
| } | ||
|
|
||
| protected: | ||
| /** | ||
| * @brief Check if the user has specified custom row bounds | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -19,6 +19,7 @@ | |
| #include <thrust/transform_scan.h> | ||
|
|
||
| #include <numeric> | ||
| #include <unordered_map> | ||
|
|
||
| namespace cudf::io::parquet::detail { | ||
|
|
||
|
|
@@ -419,29 +420,32 @@ void reader_impl::create_global_chunk_info() | |
| auto const num_chunks = row_groups_info.size() * num_input_columns; | ||
|
|
||
| // Mapping of input column to page index column | ||
| std::vector<size_type> column_mapping; | ||
|
|
||
| if (_has_offset_index and not row_groups_info.empty()) { | ||
| // use first row group to define mappings (assumes same schema for each file) | ||
| auto const& rg = row_groups_info[0]; | ||
| auto const& columns = _metadata->get_row_group(rg.index, rg.source_index).columns; | ||
| column_mapping.resize(num_input_columns); | ||
| std::transform( | ||
| _input_columns.begin(), _input_columns.end(), column_mapping.begin(), [&](auto const& col) { | ||
| // translate schema_idx into something we can use for the page indexes | ||
| if (auto it = std::find_if(columns.begin(), | ||
| columns.end(), | ||
| [&](auto const& col_chunk) { | ||
| return col_chunk.schema_idx == | ||
| _metadata->map_schema_index(col.schema_idx, | ||
| rg.source_index); | ||
| }); | ||
| it != columns.end()) { | ||
| return std::distance(columns.begin(), it); | ||
| } | ||
| CUDF_FAIL("cannot find column mapping"); | ||
| }); | ||
| } | ||
| auto column_mappings = std::unordered_map<size_type, std::vector<size_type>>{}; | ||
|
|
||
| auto const column_mapping_for_source = [&](auto const& rg) -> std::vector<size_type> const& { | ||
| auto const [iter, inserted] = column_mappings.try_emplace(rg.source_index); | ||
| if (inserted) { | ||
| auto const& columns = _metadata->get_row_group(rg.index, rg.source_index).columns; | ||
| auto& mapping = iter->second; | ||
| mapping.resize(num_input_columns); | ||
| std::transform( | ||
| _input_columns.begin(), _input_columns.end(), mapping.begin(), [&](auto const& col) { | ||
| // translate schema_idx into something we can use for the page indexes | ||
| if (auto it = std::find_if(columns.begin(), | ||
| columns.end(), | ||
| [&](auto const& col_chunk) { | ||
| return col_chunk.schema_idx == | ||
| _metadata->map_schema_index(col.schema_idx, | ||
| rg.source_index); | ||
| }); | ||
| it != columns.end()) { | ||
| return static_cast<size_type>(std::distance(columns.begin(), it)); | ||
| } | ||
| CUDF_FAIL("cannot find column mapping"); | ||
| }); | ||
| } | ||
| return iter->second; | ||
| }; | ||
|
Comment on lines
+423
to
+448
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Create per-source column mappings instead since we could have row groups from different files with column chunks sitting at different positions. This bug is unrelated to this PR but was discovered by the newly added tests so fixed it here. |
||
|
|
||
| // Initialize column chunk information | ||
| auto remaining_rows = num_rows; | ||
|
|
@@ -454,6 +458,8 @@ void reader_impl::create_global_chunk_info() | |
| auto row_group_rows = | ||
| std::min<size_t>(remaining_rows + adjusted_row_group_rows, row_group.num_rows); | ||
|
|
||
| auto const* const column_mapping = _has_offset_index ? &column_mapping_for_source(rg) : nullptr; | ||
|
|
||
| // generate ColumnChunkDesc objects for everything to be decoded (all input columns) | ||
| for (size_t i = 0; i < num_input_columns; ++i) { | ||
| auto col = _input_columns[i]; | ||
|
|
@@ -479,7 +485,7 @@ void reader_impl::create_global_chunk_info() | |
|
|
||
| // grab the column_chunk_info for each chunk (if it exists) | ||
| column_chunk_info const* const chunk_info = | ||
| _has_offset_index ? &rg.column_chunks.value()[column_mapping[i]] : nullptr; | ||
| _has_offset_index ? &rg.column_chunks.value()[(*column_mapping)[i]] : nullptr; | ||
|
|
||
| chunks.emplace_back(col_meta.total_compressed_size, | ||
| nullptr, | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Simply enable this regardless of column selection. Hybrid scan selects columns later (filter/payload/all) later on anyway so no point checking things here.