mbutrovich commented on code in PR #3298: URL: https://github.com/apache/datafusion-comet/pull/3298#discussion_r2737060413
########## spark/src/main/scala/org/apache/comet/serde/operator/CometIcebergNativeScan.scala: ########## Review Comment: Does it make sense to try to encapsulate these remaining reflection calls in the cache as well? ########## spark/src/main/scala/org/apache/comet/serde/operator/CometIcebergNativeScan.scala: ########## Review Comment: I am concerned about some of the nested try-catch logic here. If we fail to serialize delete files, we should propagate an exception to make sure we fall back to Spark. IIUC, this code will return `None` for delete files, which will results in a serialize scan that will not apply delete files and potentially generate invalid data. -- 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]
