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]

Reply via email to