andygrove commented on PR #5638:
URL: 
https://github.com/apache/datafusion-comet/pull/5638#issuecomment-5529576292

   _LLM-assisted: this reply and the changes it describes were written with 
Claude Code._
   
   @jordepic thanks for tracing it against iceberg-java, and for the framing 
about the sort key and the partition values coming from two different 
implementations — that is the sharpest statement of the risk here.
   
   I went with the test rather than the delegation, for two reasons. The 
wrapper would not get much thinner: the `Int8`/`Int16` arm and the dictionary 
handling stay either way and `days` still needs its `Date32` identity case, so 
what actually disappears is `murmur3_32` and the two `hash_*` helpers — in 
exchange for coupling the semantics to a pinned git rev when Appendix B already 
fixes them. And `years`/`months` cannot be delegated at all, since `date_part` 
honours the timezone tag.
   
   So there is now an `iceberg_rust_transform_parity` module in 
`iceberg_write.rs` (8d898c2) asserting each `SparkIceberg*` UDF matches 
`create_transform_function` over the boundary inputs of every type both sides 
accept: `bucket` over int/long/date/timestamp (tagged and 
untagged)/decimal/string/binary at four bucket counts, `truncate` at five 
widths, `days`/`hours` over the epoch boundaries, and `years`/`months` where 
iceberg-rust can represent the date. The exclusions are the divergences 
themselves and are documented there.
   
   On the dictionary unpack — you were right that it was worth measuring, and 
worth doing. `apply_unary` now runs the kernel over the dictionary's values and 
expands through the keys (1622503). Over 8192 rows with eight distinct strings:
   
   | case | before | after |
   | --- | --- | --- |
   | `iceberg_bucket/string_dict` | 72.9 µs | 3.0 µs |
   | `iceberg_truncate/string_dict` | 140.9 µs | 35.4 µs |
   
   For reference the plain string column is 31.2 µs for `bucket` and 99.1 µs 
for `truncate`, so the dictionary shape went from 2.3x / 1.4x *slower* than 
plain to faster than it, which is what you would expect once the hash runs once 
per distinct value.


-- 
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