andygrove opened a new pull request, #6990: URL: https://github.com/apache/datafusion-comet/pull/6990
## Which issue does this PR close? Closes #6978. ## Rationale for this change The root schema remap (`remap_physical_schema`) and the nested struct matcher (`match_struct_fields`) each implemented Spark's `clipParquetGroupFields` field resolution on their own: an id-bearing requested field resolves by Parquet field id only, and every other field resolves by folded name. A Spark-compatibility fix to one copy could miss the other and reintroduce a schema-evolution bug at one level only. `create` and `rewrite` also inlined several independent steps. ## What changes are included in this PR? - A new `parquet/field_resolver.rs` with a `FieldResolver` for one group, either the root schema or a struct. For each requested field it reports which file fields that field selects, by id or by folded name, the way Spark resolves each requested field on its own. `resolve` raises Spark's errors for an ambiguous selection. - `match_struct_fields`, which serves both the nested convert and the plan-time conversion check, is now a thin wrapper over `FieldResolver::resolve`. - `remap_physical_schema` asks the same resolver about each requested root field and turns the answers into one `RootResolution` per file column: `IdMatch`, `Shadowed`, `NameMatch` or `Unchanged`. A rename can give a column only one name, so a column that several requested fields claim takes the strongest claim, and the earliest requested field wins a tie. `FakeNameGenerator` and `rename_to_logical` replace the rename and placeholder code that was copied between the id and name branches. Name matching uses hash lookups instead of comparing every physical field with every logical one. - `create` and `rewrite` are split into named steps (`colliding_folded_names`, `id_resolved_folded_names`, `id_duplicate_roots`, `resolve_rebase_policies`, `check_duplicate_columns`, `restore_physical_names`), and each step keeps its comments. - `field_names_with_id` is removed. The resolver builds the duplicate-id message from the indices it matched. Behavior is unchanged except for one error-ordering detail. The root remap now folds names before it checks for duplicate field ids, so a read that hits both a JVM name-fold failure and a duplicate id reports the fold failure. The tests stay in `schema_adapter.rs`. Moving them out, the optional step 5 in the issue, is a pure move that is easier to review on its own. ## How are these changes tested? - New unit tests for each resolver rule: an id match wins over a same-named field, a missing id reads as a missing column, a field without an id resolves by name beside id-bearing siblings, ids are ignored when id matching is off, case-insensitive and case-sensitive ambiguity both fail, and ambiguity nobody requests is harmless. Two more tests cover the root precedence: `root_resolution_takes_the_strongest_claim` and `root_resolution_earliest_requested_field_wins_a_tie`. - The existing native tests (`cargo test -p datafusion-comet --lib`), `ParquetReadV1Suite` and `CometNativeReaderSuite` pass. - During development, a temporary differential test (not included here) ran the previous code beside the new code. It covered `remap_physical_schema` and `match_struct_fields` on 1.6 million random schema pairs each, and the whole adapter (`create`, then `rewrite` of every column and column pair) on 120,000 more. The schemas mixed names that collide by case, repeated names and ids, missing ids and nested structs, under every combination of case sensitivity and id matching. Results and error messages matched in every case. Two deliberately planted precedence bugs each made it fail. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
