parthchandra commented on code in PR #5377:
URL: https://github.com/apache/datafusion-comet/pull/5377#discussion_r3808678736
##########
spark/src/main/scala/org/apache/comet/rules/CometScanRule.scala:
##########
@@ -540,15 +540,40 @@ case class CometScanRule(session: SparkSession)
}
}
- // Comet serializes the whole table/scan schema to native, not just
projected columns, so a
- // type the native reader does not support (e.g. variant) breaks the
scan even when that
- // column is not projected. The readSchema allow-list only covers
projected columns, so run
- // the same allow-list over the full schema Comet may serialize.
Reflection failure also
- // falls back.
+ // The whole Iceberg table schema is serialized to native, but
iceberg-rust can represent
+ // Variant in that schema as long as no projected field contains one.
Match projected
+ // roots by field ID so historical snapshots still identify renamed
columns, check them
+ // strictly, and allow Variant only under entirely unprojected roots.
Other unsupported
+ // types still fail closed everywhere. An empty data projection is
also strict because
+ // iceberg-rust currently interprets an empty field-id list as a
request for every column.
val schemaTypesSupported =
try {
val fullSchema =
IcebergReflection.toSparkSchema(metadata.tableSchema)
- typeChecker.isSchemaSupported(fullSchema, fallbackReasons)
+ val projectedDataColumns =
scanExec.output.filterNot(_.isMetadataCol)
+ val unprojectedTypeChecker = new CometScanTypeChecker() {
Review Comment:
This is being called for all the top level variant fields. What about a
nested variant field?
##########
spark/src/test/resources/sql-tests/expressions/misc/variant.sql:
##########
@@ -22,14 +22,26 @@
-- MinSparkVersion: 4.0
statement
-CREATE TABLE test_variant(id INT, v VARIANT) USING parquet
+CREATE TABLE test_variant(id INT, v VARIANT, tail STRING) USING parquet
Review Comment:
Should we also add a test with `ARRAY<VARIANT>` and/or `MAP<STRING, VARIANT>`
--
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]
