peterxcli opened a new pull request, #5681:
URL: https://github.com/apache/datafusion-comet/pull/5681

   ## Which issue does this PR close?
   
   Closes #5671.
   
   ## Rationale for this change
   
   `SparkPhysicalExprAdapter` applied Spark's 
`SchemaColumnConvertNotSupportedException` matrix only to the top-level 
physical/logical pair. Same-shape struct/list/map pairs were wrapped in 
`CometCastColumnExpr`, whose `parquet_convert_array` cast every nested leaf 
with a plain Arrow `cast_with_options(safe: true)` — silently NULLing 
overflowing `INT64 -> int`, stringifying ints, parsing strings, wrapping 
scalars into arrays — and returned the unconverted array for pairs Arrow cannot 
cast, which then panicked in `StructArray::new`. Spark's vectorized reader runs 
`ParquetVectorUpdaterFactory.getUpdater` on every leaf regardless of nesting, 
so nested leaves must follow the same rules.
   
   ## What changes are included in this PR?
   
   - Factor the scalar rule set out of `replace_with_spark_cast` into 
`check_leaf_conversion` (identical conditions and order, so top-level behaviour 
is unchanged) returning `Accept` / `Reject` / `RejectOnNonEmpty`.
   - Add `check_conversion`, which walks same-shape struct/list/map pairs at 
any depth, resolves struct fields with the runtime convert's field-id / 
case-fold rules (new shared `match_struct_fields`, extracted from 
`parquet_convert_struct_to_struct`), applies the rules to every leaf, reports 
the Spark-style column path (`Column: [s, x]`) and honours `RejectOnNonEmpty` 
(SPARK-26709) like the top level. Missing nested fields still read as 
null/default.
   - Apply the check on both the `CastExpr` path and the default-adapter 
fallback path (`wrap_all_type_mismatches`), which previously enforced only the 
string/binary rule.
   - `parquet_convert_array`: turn the `_ => Ok(array)` fallthrough into an 
error and use `try_new` for struct/list/map so a mismatch is an error, never a 
panic.
   - `spark_catalog_name`: render `array<..>` / `struct<..>` / `map<..>` per 
Spark's `catalogString` (was `unknown`).
   
   Behaviour notes for reviewers:
   - The fallback path now applies the full rule set to its top-level pair 
(previously only the string/binary rule).
   - A complex-vs-complex shape mismatch (e.g. STRUCT read as ARRAY) and a 
dictionary-encoded physical column (files carrying an `ARROW:schema`) now get 
the Spark-shaped rejection at plan time instead of falling into a Spark `Cast`.
   - The nested case-insensitive duplicate-field error now fires at plan time 
(previously on the first batch).
   
   ## How are these changes tested?
   
   - New Rust tests in `schema_adapter.rs`, run through a real `DataSourceExec` 
+ `SparkPhysicalExprAdapterFactory`: the six cases from the issue (INT64→int, 
int→string, decimal narrowing, string→int, int→array<int>, array<int>→int — the 
former panic), list-of-struct and map-value leaves, case-insensitive and 
field-id nested matching, `RejectOnNonEmpty` for non-empty vs. empty files, and 
positives (int32→int64 with type promotion, TIMESTAMP_MILLIS→micros inside a 
list, missing nested field → null); plus a `parquet_support.rs` test that 
`array<int> -> int` inside a struct is an error rather than a panic. `cargo 
test -p datafusion-comet parquet`: 112 passed, 0 failed.
   - New Scala tests in `ParquetReadSuite`: `native scan rejects nested Parquet 
conversions Spark rejects` (asserts Spark and Comet both throw and Comet's 
cause chain contains `SchemaColumnConvertNotSupportedException` for 
struct/array/map cases), `nested schema evolution follows Spark's per-version 
widening rules`, `nested TIMESTAMP_MILLIS columns read as timestamp`. 
`ParquetReadV1Suite` + `ParquetTimestampLtzAsNtzSuite` on Spark 4.1.3: 68 
passed, 0 failed (3 canceled are pre-existing pre-Spark-4 `assume` gates).
   - `cargo fmt` / `cargo clippy --all-targets --workspace -- -D warnings` 
clean; `spotless:apply` applied.
   


-- 
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]

Reply via email to