viirya commented on issue #6499: URL: https://github.com/apache/datafusion-comet/issues/6499#issuecomment-6119082942
Thanks, these are all real gaps. Taking them in order: **Murmur3 and leaf partitioning.** You're right, I only considered both sides going through an exchange. Keeping Spark's hash inside `RepartitionExec` isn't possible, so admission will decline any plan with partitioning reported by a scan: a bucketed scan's `HashPartitioning` or the `KeyGroupedPartitioning` of a storage-partitioned join. That is broader than the mixed bucketed join, but it avoids relying on file groups lining up with buckets. I'll add the bucketed-to-non-bucketed join on the bucket column as a test that checks it is declined and matches Spark. **Partition index in shared expressions.** Agreed. The `deterministic` gate and the `nativeOnly` protobuf walk will apply to every block of the operator tree, so partition-sensitive expressions, nondeterministic expressions and JVM UDF dispatch keep the plan on the ordinary path. **Range.** The first PR will lower `RangeScan` through `create_plan` instead of adding a second range operator, so `mayOverflow` keeps the overflowing ranges on Spark. The range tests will compare against Spark's answer, including your two examples. **Session.** The local session will come from `create_execution_context` now that #6695 is in, which brings Comet's function overrides, the spill directories and their size cap, the `spark.comet.datafusion.*` pass-through and the skip-partial-aggregation guard. That also covers the spill-directory point from the #6500 review. **Budget.** Agreed, using the result task's `CometTaskMemoryManager` is better than a pool Spark never sees. A local query then draws on `spark.memory.offHeap.size` through Spark's per-task accounting like any other Comet task, concurrent local queries share that bound, and `spark.comet.exec.local.memoryLimit` goes away. The first PR will start from current main with the lifecycle, JNI bridge and result delivery, plus `RangeScan` lowered through `create_plan`, the session from `create_execution_context` and the task memory manager. -- 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]
