ziting-openai commented on code in PR #5262:
URL: https://github.com/apache/datafusion-comet/pull/5262#discussion_r3930092772
##########
native/Cargo.toml:
##########
@@ -62,8 +62,8 @@ object_store = { version = "0.13.2", features = ["gcp",
"azure", "aws", "http"]
url = "2.2"
aws-config = "1.8.18"
aws-credential-types = "1.2.13"
-iceberg = { git = "https://github.com/apache/iceberg-rust", rev =
"8adaa872f31549dd5ad8255848715758228038bc" }
-iceberg-storage-opendal = { git = "https://github.com/apache/iceberg-rust",
rev = "8adaa872f31549dd5ad8255848715758228038bc", features = ["opendal-memory",
"opendal-fs", "opendal-s3", "opendal-gcs", "opendal-oss", "opendal-azdls"] }
+iceberg = { git = "https://github.com/apache/iceberg-rust", rev =
"665c64e48e8d33797ecb1a421f327edd9b024879" }
Review Comment:
At `598f5e979bdaf82b929596dfcae6ec116bae265e`, the compiler errors are
fixed, but the new `FileScanTask` builder validation still rejects valid
serialized scan tasks:
- Metadata projections can omit a partition source from the serialized task
schema. [Spark 4.1 scan
CI](https://github.com/apache/datafusion-comet/actions/runs/33824701130/job/100880184873)
fails `SELECT id, _spec_id` on a table partitioned by `category`: the task
schema contains only `id`/`_spec_id`, and `.build()` at `planner.rs:4540`
rejects missing source column 2. The expression shards similarly fail
`COUNT(DISTINCT _file)` after pruning the partition source. These ordinary
metadata queries do not require a v1 table or dropped columns.
- A v1 table partitioned by `[identity(region), identity(category)]` can
remove the trailing category partition, write under its retained `void` spec,
then drop the category column. The serializer filters the now-unknown value but
retains the full spec because region remains; the builder rejects one value
versus two fields. This case is source-traced.
Please make the task schema, partition spec and values satisfy the new
validation while preserving metadata projection and dropped-field semantics.
Also supply partition values in
`test_unified_partition_type_merges_specs_by_descending_spec_id`; [Rust
CI](https://github.com/apache/datafusion-comet/actions/runs/33824701130/job/100875289278)
confirms its missing-values failure at `planner.rs:7103`.
These remain one Iceberg migration P2. CI tested synthetic merge
`27cfb46bbf6309823fcfa3d90a56140fe15c898f`; the relevant native code,
serializer, fixtures and dependency pins match the reviewed head. No local
execution was performed.
##########
native/Cargo.toml:
##########
@@ -35,18 +35,18 @@ license = "Apache-2.0"
edition = "2021"
# Comet uses the same minimum Rust version as DataFusion
-rust-version = "1.88"
+rust-version = "1.94.0"
[workspace.dependencies]
-arrow = { version = "58.4.0", features = ["prettyprint", "ffi", "chrono-tz"] }
-arrow-select = { version = "58.4.0" }
+arrow = { version = "59.2.0", features = ["prettyprint", "ffi", "chrono-tz"] }
+arrow-select = { version = "59.2.0" }
async-trait = { version = "0.1" }
bytes = { version = "1.11.1" }
-parquet = { version = "58.4.0", default-features = false, features =
["experimental"] }
-datafusion = { version = "54.1.0", default-features = false, features =
["unicode_expressions", "crypto_expressions", "nested_expressions", "parquet"] }
-datafusion-datasource = { version = "54.1.0" }
-datafusion-physical-expr-adapter = { version = "54.1.0" }
-datafusion-spark = { version = "54.1.0", features = ["core"] }
+parquet = { version = "59.2.0", default-features = false, features =
["experimental"] }
+datafusion = { version = "55.0.0", default-features = false, features =
["unicode_expressions", "crypto_expressions", "nested_expressions", "parquet"] }
Review Comment:
The compiler fixes pass native Clippy and bench compilation for this PR
head, but DF55/Arrow test migration still has two stale expectations:
- [Rust
CI](https://github.com/apache/datafusion-comet/actions/runs/33824701130/job/100875289278)
fails the `collect_set` negative drift assertion at `planner.rs:6962`. DF55
`ScalarValue::new_list` now casts non-empty values to the declared nested type,
as the test comment anticipated. Adapt only that obsolete sentinel and retain
positive state-schema coverage. Keep the `collect_list` coercion: its
accumulator still concatenates runtime arrays without this cast. Do not invert
both assertions or remove the shared protection.
- [Shuffle
CI](https://github.com/apache/datafusion-comet/actions/runs/33824701130/job/100880184870)
fails the two nullable-map-key tests at
`CometCelebornShuffleReaderSuite.scala:1343`. Arrow 59.3 now rejects that
schema during IPC decoding with `Map key field must not be nullable`, before
Comet's `type mismatch` check. The empty-row assertion passes; the expected
message is stale. Accept the appropriate upstream rejection while preserving
the no-output and fetch-failure checks.
These remain part of the existing DF/Arrow migration P2, not new production
correctness findings. CI tested synthetic merge
`27cfb46bbf6309823fcfa3d90a56140fe15c898f`; the relevant tests, native
decoder/validators and dependency pins match
`598f5e979bdaf82b929596dfcae6ec116bae265e`. No local tests were run.
--
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]