andygrove commented on PR #5051: URL: https://github.com/apache/datafusion-comet/pull/5051#issuecomment-5443392005
Filed the broadcast half of the Kryo work as #5510 so it can be split out if reviewers prefer. To be clear about what is and is not in this PR's scope: `CometBroadcastExchangeExec` broadcasts an `Array[ChunkedByteBuffer]`, which Spark does not register, so a native broadcast join fails under `spark.kryo.registrationRequired=true` on `main` today with no cache involved. 7e19dff20 fixes it here because the registrator this PR had to add anyway covers the same type — but it is a pre-existing bug, not one this PR introduces, and it should not have to wait on an experimental default-off feature. The two are entangled only through the shared class list (`Utils.arrowBytesKryoClasses`), so splitting is straightforward: that list plus the registrator plus the warning land first, and this PR keeps only `ArrowCachedBatchSerializer.kryoClasses` and the cache tests. #5510 spells out what a standalone fix needs, including the reason Comet cannot install `spark.kryo.registrator` itself. @viirya @sunchao happy either way — say which you would rather review and I will restructure, or leave it as is if folding it in here is fine. -- 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]
