andygrove commented on PR #5602:
URL:
https://github.com/apache/datafusion-comet/pull/5602#issuecomment-5667105701
Coming back to this one late — I was going through the open critical issues
looking for regressions from 1.0.0, and #5783 traces back to here.
The `assert_eq!(field_name_to_index_map.len(), from_fields.len())` that came
out of `parquet_convert_struct_to_struct` was unconditional, so it was also
catching the case-sensitive collision: a struct with two byte-identical sibling
names used to abort the scan. The new match arm is gated on `!case_sensitive`,
so that case now falls through to `indices[0]`, and the row multiplication the
assert was masking surfaces as wrong results instead. Writing
`named_struct('dup', id, 'dup', id + 100)` and reading it back as `s
struct<dup: bigint>`, 1.0.0 panics with `left: 1, right: 2` and main returns 6
rows where Spark returns 3. I built both to check.
So "no behavior change in case-sensitive mode" holds for name resolution,
but that assert was the one thing standing between a duplicate-name file and a
silently wrong row count.
Would it be reasonable to keep an unconditional duplicate check here, worded
so it doesn't claim case-insensitive mode, and keep the Spark-matching error
for the `!case_sensitive` path? Spark resolves the duplicate to the last child,
so eventually matching that would be better, but erroring is at least what we
did before and is much safer than where we are now.
--
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]
