0lai0 commented on PR #5932: URL: https://github.com/apache/datafusion-comet/pull/5932#issuecomment-5928989794
## Status summary Comet's scan never runs Spark's schema converter, so nothing compared a Parquet file's VARIANT annotation against the type the read asked for: a VARIANT column read as `struct<value binary, metadata binary>` silently returned the storage bytes where Spark raises `_LEGACY_ERROR_TEMP_3071`. This adds that check. Closes #5741. ### The five commits, in reading order | Commit | What it does | |---|---| | `5508366` | The check itself: `check_variant_annotation` in `schema_adapter.rs`, the error carrier, the serde and proto plumbing for `spark.sql.parquet.ignoreVariantAnnotation` | | `03f9da0` | Re-enables the Spark test the 4.1 diff was skipping for #5741 | | `8c1cb78` | Scopes the check to the requested read schema, and pairs map children by position the way `MapArray` reads them | | `96087ba` | Review fix: takes the marker from the file's annotation rather than its `ARROW:schema` hint (below) | | `a29268d` | Stops skipping the same Spark test on 4.2, whose diff landed on main in #4950 | ### Both review points are addressed @andygrove found that the physical side of the check was the reader's Arrow field, which parquet-rs builds from the `ARROW:schema` hint when a file carries one, never from the Parquet logical type. That made the marker the hint's rather than the file's, so Comet rejected a column Spark reads (hint marked, group unannotated) and read one Spark rejects (the reverse). `96087ba` reconciles the hint's Variant markers against the annotations in `EagerPageIndexReaderFactory`, before the adapter sees either, and adds a test per direction; both fail with the reconciliation disabled. It also installs the reader factory in `probe_variant_annotation`, which had been building its own `ParquetSource` without it — the reason no Variant test could have caught this. `a29268d` regenerates `dev/diffs/4.2.0.diff` from a clean `v4.2.0` clone per the Spark SQL Tests guide. The only change from the previous diff is that one hunk's removal. -- 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]
