0lai0 opened a new pull request, #5932: URL: https://github.com/apache/datafusion-comet/pull/5932
## Which issue does this PR close? Closes #5741. ## Rationale for this change Reading a VARIANT-annotated Parquet field as an ordinary `struct<value binary, metadata binary>` silently returned the storage bytes. Spark rejects that read in `ParquetToSparkSchemaConverter.convertGroupField` with `_LEGACY_ERROR_TEMP_3071` unless `spark.sql.parquet.ignoreVariantAnnotation` is set. `CometScanRule` cannot catch it: it only sees the requested schema, and a hand-written struct carries none of the `VariantMetadata` that `isVariantStruct` looks for. The annotation lives in the file, so the check has to happen in the native reader. ## What changes are included in this PR? arrow-rs surfaces the annotation as the `arrow.parquet.variant` Arrow extension type, and Comet's serde marks a requested `VariantType` the same way. `check_variant_annotation` compares the two sides symmetrically in `SparkPhysicalExprAdapterFactory::create`, so a marked request stays a legitimate Variant read and only an unmarked request against an annotated file is rejected. Matching on `value`/`metadata` child names instead would misclassify ordinary structs, which #5741 rules out. - `spark.sql.parquet.ignoreVariantAnnotation` plumbed through `NativeScanCommon`, read by key since the conf is 4.1-only while this file compiles against 3.4 through 4.1. - Gated to Spark 4.1+. The converter branch and the conf both arrived in 4.1, so on 3.4, 3.5 and 4.0 the check is off rather than inventing a failure Spark does not have. - Recurses through struct fields, list elements and map values, pairing nested fields by field id the way `spark_parquet_convert` does. - Runs at file open, before any row group, matching Spark rejecting during schema conversion. An empty file fails too. - `SparkError::ParquetVariantAnnotationMismatch` converted by the 4.x shim into Spark's `AnalysisException` wrapped in `FAILED_READ_FILE`. - `dev/diffs/4.1.3.diff` regenerated per `spark-sql-tests.md` to drop the `IgnoreComet` exclusion. Known gap, documented on `is_variant_marked`: the extension type carries no spec version, so with `ignoreVariantAnnotation=true` on a non-v1 annotation Comet reads a file Spark refuses. Both engines reject it when the conf is off, with different error classes. ## How are these changes tested? `ParquetVariantShreddingSuite / variant logical type annotation - ignore variant annotation` is no longer excluded. I ran the suite locally against Spark 4.1.3 with `ENABLE_COMET=true`: 7 tests pass, none ignored. With the check disabled it fails with `Expected exception org.apache.spark.SparkException to be thrown, but no exception was thrown`, the failure #5741 reports, so the suite is exercising this change. 15 native tests in `schema_adapter.rs` cover rejection at each nesting shape, a real `VARIANT(1)` file written through the low-level writer, `ignoreVariantAnnotation=true`, a marked Variant request not being rejected, an empty file, nested field-id resolution, and probes asserting the annotation still reaches `create` as an extension type. -- 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]
