sunchao commented on PR #5365: URL: https://github.com/apache/datafusion-comet/pull/5365#issuecomment-5442357496
@andygrove, a few factual clarifications against `7a0f2a59` for [these questions](https://github.com/apache/datafusion-comet/pull/5365#issuecomment-5441843040): - The default native feature was [discussed here](https://github.com/apache/datafusion-comet/pull/5365#issuecomment-5344589643) and [accepted here](https://github.com/apache/datafusion-comet/pull/5365#issuecomment-5346521797) to avoid requiring a custom native build. The separate `contrib-delta` comment says “zero kernel surface,” not zero Delta surface. Runtime opt-in does not imply zero binary cost; I have not verified the binary-size cost. - Coverage includes [Delta DELETE-produced DVs](https://github.com/apache/datafusion-comet/blob/7a0f2a59a575625094a9b07729b84c9c30bf0582/contrib/delta-spark/src/test/scala/org/apache/comet/contrib/delta/CometDeltaNativeScanSuite.scala#L447), not only Rust round trips. The [decoder](https://github.com/apache/datafusion-comet/blob/7a0f2a59a575625094a9b07729b84c9c30bf0582/native/core/src/execution/delta_dv.rs#L57) has explicit truncation/length/CRC error paths; this is not a comprehensive malformed-input or no-panic proof. - [DeltaScanSupport](https://github.com/apache/datafusion-comet/blob/7a0f2a59a575625094a9b07729b84c9c30bf0582/contrib/delta-spark/src/main/scala/org/apache/comet/contrib/delta/DeltaScanSupport.scala#L79) uses typed Delta APIs, not an IcebergReflection-style layer. The hook catches `NonFatal`; linkage errors deliberately propagate, so arbitrary version skew is not guaranteed to decline cleanly. - [CI is path-gated](https://github.com/apache/datafusion-comet/blob/7a0f2a59a575625094a9b07729b84c9c30bf0582/dev/ci/compute-changes.py#L248), including shared Spark/Common/build changes. Current-head workflows are approval-blocked, so they do not establish wall-clock or runner-minute cost. - Benchmark numbers are present in the description, but the 1-TB figures are author-reported at older commit `7fd81aa9`, with a row-reader control. A [DV plus page-pruning regression](https://github.com/apache/datafusion-comet/blob/7a0f2a59a575625094a9b07729b84c9c30bf0582/contrib/delta-spark/src/test/scala/org/apache/comet/contrib/delta/CometDeltaNativeScanSuite.scala#L568) already checks native execution, 90 surviving rows and positive pruning metrics; that is correctness coverage, not a pruning-on/off performance comparison. These are source/discussion clarifications; I did not rerun the tests or benchmarks for this follow-up. -- 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]
