rich7420 commented on code in PR #7028:
URL: https://github.com/apache/datafusion-comet/pull/7028#discussion_r4390550392
##########
native/Cargo.toml:
##########
@@ -85,8 +85,8 @@ aws-sdk-sts = { version = "1.114.0", default-features = false
}
# Pinned to a revision rather than a release because no published iceberg-rust
# version is on our arrow: 0.10.1 requires arrow/parquet ^58. Move to a plain
# version requirement once a release ships on arrow 59. Bump policy: #5645.
-iceberg = { git = "https://github.com/apache/iceberg-rust";, rev =
"1f3bc34058e3f3e45336bc5428f37b92649a608b" }
-iceberg-storage-opendal = { git = "https://github.com/apache/iceberg-rust";,
rev = "1f3bc34058e3f3e45336bc5428f37b92649a608b", features = ["opendal-memory",
"opendal-fs", "opendal-s3", "opendal-gcs", "opendal-oss", "opendal-azdls"] }
+iceberg = { git = "https://github.com/apache/iceberg-rust";, rev =
"b154603a710d730d2f43f8af4db257cc137237ef" }
+iceberg-storage-opendal = { git = "https://github.com/apache/iceberg-rust";,
rev = "b154603a710d730d2f43f8af4db257cc137237ef", features = ["opendal-memory",
"opendal-fs", "opendal-s3", "opendal-gcs", "opendal-oss", "opendal-azdls"] }
Review Comment:
Please preserve `FixedSizeList`→`List` conversion; Parquet’s Arrow schema
hint can expose this newly rejected input.
##########
spark/src/main/scala/org/apache/comet/rules/CometScanRule.scala:
##########
@@ -850,18 +851,17 @@ case class CometScanRule(session: SparkSession)
} else {
resolvedProjectedFieldIds
}
- val changed = schemas
- .flatMap { schema =>
- IcebergReflection.nestedFieldsAddedOrRenamed(metadata.table,
schema, fieldIds)
- }
+ val disjoint = metadata.taskSchema.toSeq
+
.flatMap(IcebergReflection.structsWithNoFieldsInCommon(metadata.table, _,
fieldIds))
Review Comment:
Could default validation use the task schema? With pruning disabled,
unprojected nested defaults now reach unsupported native reads.
##########
spark/src/main/scala/org/apache/comet/rules/CometScanRule.scala:
##########
@@ -850,18 +851,17 @@ case class CometScanRule(session: SparkSession)
} else {
resolvedProjectedFieldIds
}
- val changed = schemas
- .flatMap { schema =>
- IcebergReflection.nestedFieldsAddedOrRenamed(metadata.table,
schema, fieldIds)
- }
+ val disjoint = metadata.taskSchema.toSeq
Review Comment:
Please preserve fallback for evolved ID-less structs: adding `s.b` now
throws, while renames can expose incorrectly matched values.
##########
native/Cargo.toml:
##########
@@ -85,8 +85,8 @@ aws-sdk-sts = { version = "1.114.0", default-features = false
}
# Pinned to a revision rather than a release because no published iceberg-rust
# version is on our arrow: 0.10.1 requires arrow/parquet ^58. Move to a plain
# version requirement once a release ships on arrow 59. Bump policy: #5645.
-iceberg = { git = "https://github.com/apache/iceberg-rust";, rev =
"1f3bc34058e3f3e45336bc5428f37b92649a608b" }
-iceberg-storage-opendal = { git = "https://github.com/apache/iceberg-rust";,
rev = "1f3bc34058e3f3e45336bc5428f37b92649a608b", features = ["opendal-memory",
"opendal-fs", "opendal-s3", "opendal-gcs", "opendal-oss", "opendal-azdls"] }
+iceberg = { git = "https://github.com/apache/iceberg-rust";, rev =
"b154603a710d730d2f43f8af4db257cc137237ef" }
Review Comment:
Could equality deletes bind case-sensitively? This pin rejects `id`/`ID`
because Comet still sets `case_sensitive=false`.
--
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]
