Improve struct pruning logic for Parquet Schema Filtering #2733
Conversation
Signed-off-by: Niranjan Artal <[email protected]>
Signed-off-by: Niranjan Artal <[email protected]>
|
build |
| // We add the Struct to schema_map (with a placeholder for child count). | ||
| // But we might remove it later if no children match. This ensures parent-before-children | ||
| // ordering in schema_map. |
| // No match was found so skip the child. | ||
| // No match was found so skip the child |
There was a problem hiding this comment.
Absolute nit: We are inconsistent with punctuation, in this change. Note that line#235 has a period, but we opt to remove the period here.
My vote would be to punctuate deliberately, and add the period back.
| ++schema_num_children[our_num_children_index]; | ||
| // Record the current size of schema_map before processing the child | ||
| // This helps determine if the child was retained after processing. | ||
| std::size_t before_child_size = schema_map.size(); |
There was a problem hiding this comment.
Nit: We can get away with making this const.
| std::size_t before_child_size = schema_map.size(); | |
| auto const before_child_size = schema_map.size(); |
There was a problem hiding this comment.
Thanks @mythrocks for the review. Updated it. PTAL.
Signed-off-by: Niranjan Artal <[email protected]>
Thanks @revans2 for the review. For struct schema evolution issues, we don't need any changes on the java side. I tested with multiple nested structs and it works fine. We can enable the tests mentioned in the description once this PR is merged and we have a new jar including this. |
|
build |
This PR refines the logic used to prune Parquet schemas when filtering out columns that do not match the query. Previously, an entire struct could be added prematurely to the schema_map and remain there even if none of its children matched. This could result in an incorrect schema when reading Parquet files.
This pull request includes updates to the
NativeParquetJni.cppfile to improve the handling of schema pruning in thecolumn_prunerclass. The changes focus on ensuring accurate child count updates and proper struct inclusion or exclusion based on the presence of matching children.Improvements to schema pruning logic:
schema_map.schema_mapbefore and after processing each child. This helps determine if a child was retained and accurately updates the child count.schema_mapandschema_num_childrenif none of their children are included, ensuring that only relevant structs are retained.I tested this PR with these issues - NVIDIA/spark-rapids#11619, NVIDIA/spark-rapids#11620, NVIDIA/spark-rapids#11621, NVIDIA/spark-rapids#11628 and NVIDIA/spark-rapids#11629. I don't see the test failures anymore. I will close those issues after enabling the tests which would require the PR in spark-rapids.