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

Reply via email to