(datafusion-comet) branch main updated: fix: isCometEnabled name conflict (#1569)

[ka...@apache.org]

This is an automated email from the ASF dual-hosted git repository.

kazuyukitanimura pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/datafusion-comet.git


The following commit(s) were added to refs/heads/main by this push:
     new c58f26162 fix: isCometEnabled name conflict (#1569)
c58f26162 is described below

commit c58f26162dc8bc6375ac228b88f2da2b1bb686a8
Author: KAZUYUKI TANIMURA <kt...@apple.com>
AuthorDate: Tue Mar 25 18:21:14 2025 -0700

    fix: isCometEnabled name conflict (#1569)
    
    ## Which issue does this PR close?
    
    ## Rationale for this change
    
    We have a name collision on `isCometEnabled`
    
    ```
    CometSparkSessionExtensions
    private[comet] def isCometEnabled(conf: SQLConf): Boolean = {
    ```
    and
    ```
    SQLTestUtilsBase
    protected def isCometEnabled: Boolean = SparkSession.isCometEnabled
    ```
    
    causing build issues depending on which one gets picked up
    ```
    [ERROR] /workspace/spark/src/test/scala/org/apache/comet/CometSparkSessionExtensionsSuite.scala:34: Boolean does not take parameters
    [ERROR] /workspace/spark/src/test/scala/org/apache/comet/CometSparkSessionExtensionsSuite.scala:42: Boolean does not take parameters
    [ERROR] /workspace/spark/src/test/scala/org/apache/comet/CometSparkSessionExtensionsSuite.scala:47: Boolean does not take parameters
    ```
    
    ## What changes are included in this PR?
    
    Renamed one to `isCometLoaded`
    
    ## How are these changes tested?
    
    CI
---
 .../org/apache/comet/CometSparkSessionExtensions.scala     | 14 +++++++-------
 .../apache/comet/CometSparkSessionExtensionsSuite.scala    |  8 ++++----
 2 files changed, 11 insertions(+), 11 deletions(-)

diff --git a/spark/src/main/scala/org/apache/comet/CometSparkSessionExtensions.scala b/spark/src/main/scala/org/apache/comet/CometSparkSessionExtensions.scala
index 0b423d348..735363e72 100644
--- a/spark/src/main/scala/org/apache/comet/CometSparkSessionExtensions.scala
+++ b/spark/src/main/scala/org/apache/comet/CometSparkSessionExtensions.scala
@@ -55,7 +55,7 @@ import org.apache.spark.sql.types.{DoubleType, FloatType}
 
 import org.apache.comet.CometConf._
 import org.apache.comet.CometExplainInfo.getActualPlan
-import org.apache.comet.CometSparkSessionExtensions.{createMessage, getCometBroadcastNotEnabledReason, getCometShuffleNotEnabledReason, isANSIEnabled, isCometBroadCastForceEnabled, isCometEnabled, isCometExecEnabled, isCometJVMShuffleMode, isCometNativeShuffleMode, isCometScan, isCometScanEnabled, isCometShuffleEnabled, isSpark40Plus, shouldApplySparkToColumnar, withInfo, withInfos}
+import org.apache.comet.CometSparkSessionExtensions.{createMessage, getCometBroadcastNotEnabledReason, getCometShuffleNotEnabledReason, isANSIEnabled, isCometBroadCastForceEnabled, isCometExecEnabled, isCometJVMShuffleMode, isCometLoaded, isCometNativeShuffleMode, isCometScan, isCometScanEnabled, isCometShuffleEnabled, isSpark40Plus, shouldApplySparkToColumnar, withInfo, withInfos}
 import org.apache.comet.parquet.{CometParquetScan, SupportsComet}
 import org.apache.comet.rules.RewriteJoin
 import org.apache.comet.serde.OperatorOuterClass.Operator
@@ -93,8 +93,8 @@ class CometSparkSessionExtensions
 
   case class CometScanRule(session: SparkSession) extends Rule[SparkPlan] {
     override def apply(plan: SparkPlan): SparkPlan = {
-      if (!isCometEnabled(conf) || !isCometScanEnabled(conf)) {
-        if (!isCometEnabled(conf)) {
+      if (!isCometLoaded(conf) || !isCometScanEnabled(conf)) {
+        if (!isCometLoaded(conf)) {
           withInfo(plan, "Comet is not enabled")
         } else if (!isCometScanEnabled(conf)) {
           withInfo(plan, "Comet Scan is not enabled")
@@ -975,8 +975,8 @@ class CometSparkSessionExtensions
         }
       }
 
-      // We shouldn't transform Spark query plan if Comet is disabled.
-      if (!isCometEnabled(conf)) return plan
+      // We shouldn't transform Spark query plan if Comet is not loaded.
+      if (!isCometLoaded(conf)) return plan
 
       if (!isCometExecEnabled(conf)) {
         // Comet exec is disabled, but for Spark shuffle, we still can use Comet columnar shuffle
@@ -1172,9 +1172,9 @@ object CometSparkSessionExtensions extends Logging {
   }
 
   /**
-   * Checks whether Comet extension should be enabled for Spark.
+   * Checks whether Comet extension should be loaded for Spark.
    */
-  private[comet] def isCometEnabled(conf: SQLConf): Boolean = {
+  private[comet] def isCometLoaded(conf: SQLConf): Boolean = {
     if (isBigEndian) {
       logInfo("Comet extension is disabled because platform is big-endian")
       return false
diff --git a/spark/src/test/scala/org/apache/comet/CometSparkSessionExtensionsSuite.scala b/spark/src/test/scala/org/apache/comet/CometSparkSessionExtensionsSuite.scala
index 33552a116..58fa69ae8 100644
--- a/spark/src/test/scala/org/apache/comet/CometSparkSessionExtensionsSuite.scala
+++ b/spark/src/test/scala/org/apache/comet/CometSparkSessionExtensionsSuite.scala
@@ -27,11 +27,11 @@ class CometSparkSessionExtensionsSuite extends CometTestBase {
 
   import CometSparkSessionExtensions._
 
-  test("isCometEnabled") {
+  test("isCometLoaded") {
     val conf = new SQLConf
 
     conf.setConfString(CometConf.COMET_ENABLED.key, "false")
-    assert(!isCometEnabled(conf))
+    assert(!isCometLoaded(conf))
 
     // Since the native lib is probably already loaded due to previous tests, we reset it here
     NativeBase.setLoaded(false)
@@ -39,12 +39,12 @@ class CometSparkSessionExtensionsSuite extends CometTestBase {
     conf.setConfString(CometConf.COMET_ENABLED.key, "true")
     val oldProperty = System.getProperty("os.name")
     System.setProperty("os.name", "foo")
-    assert(!isCometEnabled(conf))
+    assert(!isCometLoaded(conf))
 
     System.setProperty("os.name", oldProperty)
 
     conf.setConf(SQLConf.PARQUET_INT96_TIMESTAMP_CONVERSION, true)
-    assert(!isCometEnabled(conf))
+    assert(!isCometLoaded(conf))
 
     // Restore the original state
     NativeBase.setLoaded(true)


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@datafusion.apache.org
For additional commands, e-mail: commits-help@datafusion.apache.org