comphead commented on code in PR #6725:
URL: https://github.com/apache/datafusion-comet/pull/6725#discussion_r4285181387
##########
spark/src/main/scala/org/apache/comet/iceberg/IcebergReflection.scala:
##########
@@ -2358,11 +2440,24 @@ case class CometIcebergNativeScanMetadata(
nameMapping: Option[String],
@transient tasks: java.util.List[_],
scanSchema: Any,
+ prunedTaskSchema: Option[Any],
+ fullTaskSchema: Option[Any],
tableSchema: Any,
globalFieldIdMapping: Map[String, Int],
catalogProperties: Map[String, String],
catalogName: Option[String],
- fileFormat: String)
+ fileFormat: String) {
+
+ /**
+ * The schema every task of the scan reads with: the pruned task schema
while nested schema
+ * pruning is on and can apply, the full one otherwise. CometScanRule checks
this schema and
+ * CometIcebergNativeScan serializes it, so the two cannot disagree. None
falls back to Spark.
+ */
+ def taskSchema: Option[Any] =
Review Comment:
Confirmed from the code, thanks. `serializedPartitionData` is a lazy val in
`CometIcebergNativeScanExec`, so `serializePartitions` reads `taskSchema`, and
the config with it, only when the scan runs, after `CometScanRule` read it
during planning. 170e5eb48d makes `taskSchema` a `val`, so the choice is fixed
when `extract` builds the metadata, which only happens during planning. The
struct fallback test now plans your query with pruning on and collects it with
pruning off. With the old `def`, that collect fails with the error you saw,
`Incorrect number of arrays for StructArray fields, expected 2 got 1`.
##########
spark/src/test/scala/org/apache/comet/CometIcebergNativeSuite.scala:
##########
@@ -1801,22 +1827,61 @@ class CometIcebergNativeSuite
checkNestedFieldEvolutionFallback(
s"SELECT id FROM $table WHERE s IS NULL ORDER BY id",
"s.b (added)")
- // The native read asks for the column's full nested type even when
Spark prunes it.
+ // The native read asks only for s.a here, but the check also covers
the column's full
+ // type in the table schema, so it still falls back.
checkNestedFieldEvolutionFallback(
s"SELECT id, s.a FROM $table ORDER BY id",
"s.b (added)")
checkNestedFieldEvolutionFallback(
s"SELECT l.id, r.id FROM $table l JOIN $table r ON l.s = r.s ORDER
BY l.id",
"s.b (added)")
- // The old snapshot's files lack s.b too, and the native read uses the
current schema.
+ // The old snapshot's files lack s.b too, and the scan's full schema
is the current one,
+ // which has it.
checkNestedFieldEvolutionFallback(
s"SELECT id, s FROM $table VERSION AS OF $snapshotBeforeAdd ORDER BY
id",
"s.b (added)")
// A read that does not project the evolved column stays native.
checkIcebergNativeScan(s"SELECT id FROM $table ORDER BY id")
- spark.sql(s"DROP TABLE $table")
+ // A VERSION AS OF read whose full schema is older than the table's:
the pruned-away
Review Comment:
Agreed, and thanks for checking it on `main`. I filed #7019 for merging
`s.k` back into its pruned struct. #6934 overlaps: once iceberg-rust matches
nested fields by id and the nested-evolution check narrows, this read goes
native again, but through the full schema, so it decodes every leaf of `s`.
#7019 would keep the pruned read, without waiting for the bump.
--
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]
