dwsmith1983 commented on code in PR #5365:
URL: https://github.com/apache/datafusion-comet/pull/5365#discussion_r3886372102
##########
native/core/src/parquet/schema_adapter.rs:
##########
@@ -193,11 +688,16 @@ fn remap_physical_schema(
}
// Block accidental name match for ID-bearing logical fields whose
ID is missing
- // from the file. Mirrors Spark's `generateFakeColumnName` in
`matchIdField`.
+ // from the file. Mirrors Spark's `generateFakeColumnName` in
`matchIdField`. Runs
+ // regardless of `case_sensitive` (field-ID matching is
independent of Spark's
+ // casing config), so `case_tables` may be absent here even when
it would be
+ // populated for a genuinely case-insensitive scan --
`names_equal_ignore_case_java`
+ // degrades to `str::to_lowercase` in that case, which is fine for
this defensive,
+ // over-approximating check (see `java_lowercase`).
if should_match_by_id
&& unmatched_id_logical_names
.iter()
- .any(|name| name.eq_ignore_ascii_case(field.name()))
+ .any(|name| names_equal_ignore_case_java(name,
field.name(), case_tables))
Review Comment:
Fixed, the shield compares exactly under case sensitivity and only fires for
fields the name pass didn't claim, matching clipParquetGroupFields. Auditing
the neighboring comparisons turned up four more divergences from Spark's
semantics (a stray ID-less column could win an exact-name lookup for an
ID-matched field, the fake name could collide with a real column, and two
duplicate-handling mismatches), all fixed with tests against real files.
One thing from that audit worth your eyes separately:
validateIcebergFileScanTasks keys on a hardcoded scheme list including gcs,
oss, wasb, and wasbs, which the native url parser doesn't recognize, so those
Iceberg tables get claimed and then fail at execution. Left untouched here
since it's the Iceberg path, but it's the same class of drift this PR kept
hitting.
--
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]
