peterxcli commented on PR #5407: URL: https://github.com/apache/datafusion-comet/pull/5407#issuecomment-5447548558
On the `pushVariantIntoScan` default, @andygrove — confirmed against Spark source: `spark.sql.variant.pushVariantIntoScan` defaults to `false` on 4.0.1 and `true` (internal) on 4.1.0, 4.1.3, and 4.2.0, so on 4.1+ the pushed rewrite makes projecting scans fall back out of the box. Addressed: - The fallback reason now names the actual lever — `rewritten by spark.sql.variant.pushVariantIntoScan; not supported by native scan` — instead of the misleading "shredded" (0cc63a7c9, with the `variant.sql` assertion updated). - The datatypes user guide now documents `VariantType` support with the Spark 4.1+ conf requirement. - The description now states the requirement, and the benchmark environment is labelled with `spark.sql.variant.pushVariantIntoScan=false` (the 4.1.3 runs were made with it set). On recognizing the one-field pushed struct: agreed it is semantically exactly whole-value projection (`RequestedVariantField.fullVariant`, path `$`, target `VariantType`), but the scan's output contract differs — `struct<0: variant>` with per-field `VariantMetadata`, consumed by ordinal through Spark's rewritten expressions — so supporting it means admitting a second output shape through the scan gate, the FFI boundary, and the JVM vector mapping. I would rather keep this PR's admission surface narrow and do that as the immediate next step: filed #5519 to track it, linked from the description and the user docs. -- 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]
