andygrove commented on code in PR #509: URL: https://github.com/apache/datafusion-comet/pull/509#discussion_r1628307927
########## spark/src/main/scala/org/apache/comet/CometSparkSessionExtensions.scala: ########## @@ -712,17 +712,6 @@ class CometSparkSessionExtensions } override def apply(plan: SparkPlan): SparkPlan = { - // DataFusion doesn't have ANSI mode. For now we just disable CometExec if ANSI mode is - // enabled. - if (isANSIEnabled(conf)) { - if (COMET_ANSI_MODE_ENABLED.get()) { - logWarning("Using Comet's experimental support for ANSI mode.") - } else { - logInfo("Comet extension disabled for ANSI mode") - return plan - } - } Review Comment: This current check will make all plans fall back to Spark if ANSI is enabled, so no native plans will run, unless `COMET_ANSI_MODE_ENABLED` is enabled. The changes in this PR mean that we still have the same check but only if the plan actually contains any expressions that would be affected by enabling ANSI support and we still fall back to Spark by default unless `COMET_ANSI_MODE_ENABLED` is enabled. The main risk with this PR is if we don't have the complete list of expressions that support ANSI mode. -- 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: github-unsubscr...@datafusion.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org --------------------------------------------------------------------- To unsubscribe, e-mail: github-unsubscr...@datafusion.apache.org For additional commands, e-mail: github-h...@datafusion.apache.org