Skip to content

Commit c58f261

Browse files
fix: isCometEnabled name conflict (apache#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
1 parent 49fa287 commit c58f261

File tree

2 files changed

+11
-11
lines changed

2 files changed

+11
-11
lines changed

spark/src/main/scala/org/apache/comet/CometSparkSessionExtensions.scala

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -55,7 +55,7 @@ import org.apache.spark.sql.types.{DoubleType, FloatType}
5555

5656
import org.apache.comet.CometConf._
5757
import org.apache.comet.CometExplainInfo.getActualPlan
58-
import org.apache.comet.CometSparkSessionExtensions.{createMessage, getCometBroadcastNotEnabledReason, getCometShuffleNotEnabledReason, isANSIEnabled, isCometBroadCastForceEnabled, isCometEnabled, isCometExecEnabled, isCometJVMShuffleMode, isCometNativeShuffleMode, isCometScan, isCometScanEnabled, isCometShuffleEnabled, isSpark40Plus, shouldApplySparkToColumnar, withInfo, withInfos}
58+
import org.apache.comet.CometSparkSessionExtensions.{createMessage, getCometBroadcastNotEnabledReason, getCometShuffleNotEnabledReason, isANSIEnabled, isCometBroadCastForceEnabled, isCometExecEnabled, isCometJVMShuffleMode, isCometLoaded, isCometNativeShuffleMode, isCometScan, isCometScanEnabled, isCometShuffleEnabled, isSpark40Plus, shouldApplySparkToColumnar, withInfo, withInfos}
5959
import org.apache.comet.parquet.{CometParquetScan, SupportsComet}
6060
import org.apache.comet.rules.RewriteJoin
6161
import org.apache.comet.serde.OperatorOuterClass.Operator
@@ -93,8 +93,8 @@ class CometSparkSessionExtensions
9393

9494
case class CometScanRule(session: SparkSession) extends Rule[SparkPlan] {
9595
override def apply(plan: SparkPlan): SparkPlan = {
96-
if (!isCometEnabled(conf) || !isCometScanEnabled(conf)) {
97-
if (!isCometEnabled(conf)) {
96+
if (!isCometLoaded(conf) || !isCometScanEnabled(conf)) {
97+
if (!isCometLoaded(conf)) {
9898
withInfo(plan, "Comet is not enabled")
9999
} else if (!isCometScanEnabled(conf)) {
100100
withInfo(plan, "Comet Scan is not enabled")
@@ -975,8 +975,8 @@ class CometSparkSessionExtensions
975975
}
976976
}
977977

978-
// We shouldn't transform Spark query plan if Comet is disabled.
979-
if (!isCometEnabled(conf)) return plan
978+
// We shouldn't transform Spark query plan if Comet is not loaded.
979+
if (!isCometLoaded(conf)) return plan
980980

981981
if (!isCometExecEnabled(conf)) {
982982
// Comet exec is disabled, but for Spark shuffle, we still can use Comet columnar shuffle
@@ -1172,9 +1172,9 @@ object CometSparkSessionExtensions extends Logging {
11721172
}
11731173

11741174
/**
1175-
* Checks whether Comet extension should be enabled for Spark.
1175+
* Checks whether Comet extension should be loaded for Spark.
11761176
*/
1177-
private[comet] def isCometEnabled(conf: SQLConf): Boolean = {
1177+
private[comet] def isCometLoaded(conf: SQLConf): Boolean = {
11781178
if (isBigEndian) {
11791179
logInfo("Comet extension is disabled because platform is big-endian")
11801180
return false

spark/src/test/scala/org/apache/comet/CometSparkSessionExtensionsSuite.scala

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -27,24 +27,24 @@ class CometSparkSessionExtensionsSuite extends CometTestBase {
2727

2828
import CometSparkSessionExtensions._
2929

30-
test("isCometEnabled") {
30+
test("isCometLoaded") {
3131
val conf = new SQLConf
3232

3333
conf.setConfString(CometConf.COMET_ENABLED.key, "false")
34-
assert(!isCometEnabled(conf))
34+
assert(!isCometLoaded(conf))
3535

3636
// Since the native lib is probably already loaded due to previous tests, we reset it here
3737
NativeBase.setLoaded(false)
3838

3939
conf.setConfString(CometConf.COMET_ENABLED.key, "true")
4040
val oldProperty = System.getProperty("os.name")
4141
System.setProperty("os.name", "foo")
42-
assert(!isCometEnabled(conf))
42+
assert(!isCometLoaded(conf))
4343

4444
System.setProperty("os.name", oldProperty)
4545

4646
conf.setConf(SQLConf.PARQUET_INT96_TIMESTAMP_CONVERSION, true)
47-
assert(!isCometEnabled(conf))
47+
assert(!isCometLoaded(conf))
4848

4949
// Restore the original state
5050
NativeBase.setLoaded(true)

0 commit comments

Comments
 (0)