Skip to content

Commit 076fa74

Browse files
parthchandraclaude
parthchandra
and
claude
authored
chore: remove legacy ENABLE_COMET_SCAN_ONLY and ENABLE_COMET_ANSI_MODE env vars from Spark diffs (apache#4252)
* chore: remove legacy ENABLE_COMET_SCAN_ONLY and ENABLE_COMET_ANSI_MODE env vars from Spark diffs These env vars are no longer set when running Spark SQL tests. ENABLE_COMET_SCAN_ONLY is removed from all diffs (always enable exec+shuffle). ENABLE_COMET_ANSI_MODE is removed from 3.4 and 3.5 diffs (ANSI mode has dedicated tests in the Spark SQL suite). Closes apache#2724 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix: correct hunk header line counts in Spark diffs Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
1 parent a2fd51b commit 076fa74

4 files changed

Lines changed: 72 additions & 200 deletions

File tree

‎dev/diffs/3.4.3.diff‎

Lines changed: 18 additions & 60 deletions
Original file line numberDiff line numberDiff line change
@@ -1998,9 +1998,9 @@ index 104b4e416cd..b8af360fa14 100644
19981998
// Note that, if record level filtering is enabled, it should be a single record.
19991999
// If no filter is pushed down to Parquet, it should be the total length of data.
20002000
- assert(actual > 1 && actual < data.length)
2001-
+ // Only enable Comet test iff it's scan only, since with native execution
2001+
+ // Skip when Comet is enabled since with native execution
20022002
+ // `stripSparkFilter` can't remove the native filter
2003-
+ if (!isCometEnabled || isCometScanOnly) {
2003+
+ if (!isCometEnabled) {
20042004
+ assert(actual > 1 && actual < data.length)
20052005
+ }
20062006
}
@@ -2011,9 +2011,9 @@ index 104b4e416cd..b8af360fa14 100644
20112011
// Note that, if record level filtering is enabled, it should be a single record.
20122012
// If no filter is pushed down to Parquet, it should be the total length of data.
20132013
- assert(actual > 1 && actual < data.length)
2014-
+ // Only enable Comet test iff it's scan only, since with native execution
2014+
+ // Skip when Comet is enabled since with native execution
20152015
+ // `stripSparkFilter` can't remove the native filter
2016-
+ if (!isCometEnabled || isCometScanOnly) {
2016+
+ if (!isCometEnabled) {
20172017
+ assert(actual > 1 && actual < data.length)
20182018
+ }
20192019
}
@@ -2926,32 +2926,14 @@ index dd55fcfe42c..99bc018008a 100644
29262926
if (testTags.exists(_.isInstanceOf[DisableAdaptiveExecution])) {
29272927
super.test(testName, testTags: _*) {
29282928
withSQLConf(SQLConf.ADAPTIVE_EXECUTION_ENABLED.key -> "false") {
2929-
@@ -242,6 +272,29 @@ private[sql] trait SQLTestUtilsBase
2929+
@@ -242,6 +272,11 @@ private[sql] trait SQLTestUtilsBase
29302930
protected override def _sqlContext: SQLContext = self.spark.sqlContext
29312931
}
29322932

29332933
+ /**
29342934
+ * Whether Comet extension is enabled
29352935
+ */
29362936
+ protected def isCometEnabled: Boolean = SparkSession.isCometEnabled
2937-
+
2938-
+ /**
2939-
+ * Whether to enable ansi mode This is only effective when
2940-
+ * [[isCometEnabled]] returns true.
2941-
+ */
2942-
+ protected def enableCometAnsiMode: Boolean = {
2943-
+ val v = System.getenv("ENABLE_COMET_ANSI_MODE")
2944-
+ v != null && v.toBoolean
2945-
+ }
2946-
+
2947-
+ /**
2948-
+ * Whether Spark should only apply Comet scan optimization. This is only effective when
2949-
+ * [[isCometEnabled]] returns true.
2950-
+ */
2951-
+ protected def isCometScanOnly: Boolean = {
2952-
+ val v = System.getenv("ENABLE_COMET_SCAN_ONLY")
2953-
+ v != null && v.toBoolean
2954-
+ }
29552937
+
29562938
protected override def withSQLConf(pairs: (String, String)*)(f: => Unit): Unit = {
29572939
SparkSession.setActiveSession(spark)
@@ -2969,7 +2951,7 @@ diff --git a/sql/core/src/test/scala/org/apache/spark/sql/test/SharedSparkSessio
29692951
index ed2e309fa07..a5ea58146ad 100644
29702952
--- a/sql/core/src/test/scala/org/apache/spark/sql/test/SharedSparkSession.scala
29712953
+++ b/sql/core/src/test/scala/org/apache/spark/sql/test/SharedSparkSession.scala
2972-
@@ -74,6 +74,31 @@ trait SharedSparkSessionBase
2954+
@@ -74,6 +74,20 @@ trait SharedSparkSessionBase
29732955
// this rule may potentially block testing of other optimization rules such as
29742956
// ConstantPropagation etc.
29752957
.set(SQLConf.OPTIMIZER_EXCLUDED_RULES.key, ConvertToLocalRelation.ruleName)
@@ -2980,23 +2962,12 @@ index ed2e309fa07..a5ea58146ad 100644
29802962
+ .set("spark.comet.enabled", "true")
29812963
+ .set("spark.comet.parquet.respectFilterPushdown", "true")
29822964
+
2983-
+ if (!isCometScanOnly) {
2984-
+ conf
2985-
+ .set("spark.comet.exec.enabled", "true")
2986-
+ .set("spark.shuffle.manager",
2987-
+ "org.apache.spark.sql.comet.execution.shuffle.CometShuffleManager")
2988-
+ .set("spark.comet.exec.shuffle.enabled", "true")
2989-
+ .set("spark.comet.memoryOverhead", "10g")
2990-
+ } else {
2991-
+ conf
2992-
+ .set("spark.comet.exec.enabled", "false")
2993-
+ .set("spark.comet.exec.shuffle.enabled", "false")
2994-
+ }
2995-
+
2996-
+ if (enableCometAnsiMode) {
2997-
+ conf
2998-
+ .set("spark.sql.ansi.enabled", "true")
2999-
+ }
2965+
+ conf
2966+
+ .set("spark.comet.exec.enabled", "true")
2967+
+ .set("spark.shuffle.manager",
2968+
+ "org.apache.spark.sql.comet.execution.shuffle.CometShuffleManager")
2969+
+ .set("spark.comet.exec.shuffle.enabled", "true")
2970+
+ .set("spark.comet.memoryOverhead", "10g")
30002971
+ }
30012972
conf.set(
30022973
StaticSQLConf.WAREHOUSE_PATH,
@@ -3093,7 +3064,7 @@ diff --git a/sql/hive/src/test/scala/org/apache/spark/sql/hive/test/TestHive.sca
30933064
index 07361cfdce9..97dab2a3506 100644
30943065
--- a/sql/hive/src/test/scala/org/apache/spark/sql/hive/test/TestHive.scala
30953066
+++ b/sql/hive/src/test/scala/org/apache/spark/sql/hive/test/TestHive.scala
3096-
@@ -55,25 +55,54 @@ object TestHive
3067+
@@ -55,25 +55,41 @@ object TestHive
30973068
new SparkContext(
30983069
System.getProperty("spark.sql.test.master", "local[1]"),
30993070
"TestSQLContext",
@@ -3140,24 +3111,11 @@ index 07361cfdce9..97dab2a3506 100644
31403111
+ .set("spark.sql.extensions", "org.apache.comet.CometSparkSessionExtensions")
31413112
+ .set("spark.comet.enabled", "true")
31423113
+
3143-
+ val v = System.getenv("ENABLE_COMET_SCAN_ONLY")
3144-
+ if (v == null || !v.toBoolean) {
3145-
+ conf
3146-
+ .set("spark.comet.exec.enabled", "true")
3147-
+ .set("spark.shuffle.manager",
3148-
+ "org.apache.spark.sql.comet.execution.shuffle.CometShuffleManager")
3149-
+ .set("spark.comet.exec.shuffle.enabled", "true")
3150-
+ } else {
3151-
+ conf
3152-
+ .set("spark.comet.exec.enabled", "false")
3153-
+ .set("spark.comet.exec.shuffle.enabled", "false")
3154-
+ }
3155-
+
3156-
+ val a = System.getenv("ENABLE_COMET_ANSI_MODE")
3157-
+ if (a != null && a.toBoolean) {
3158-
+ conf
3159-
+ .set("spark.sql.ansi.enabled", "true")
3160-
+ }
3114+
+ conf
3115+
+ .set("spark.comet.exec.enabled", "true")
3116+
+ .set("spark.shuffle.manager",
3117+
+ "org.apache.spark.sql.comet.execution.shuffle.CometShuffleManager")
3118+
+ .set("spark.comet.exec.shuffle.enabled", "true")
31613119
+ }
31623120

31633121
+ conf

‎dev/diffs/3.5.8.diff‎

Lines changed: 18 additions & 60 deletions
Original file line numberDiff line numberDiff line change
@@ -1979,9 +1979,9 @@ index 8e88049f51e..20d7ef7b1bc 100644
19791979
// Note that, if record level filtering is enabled, it should be a single record.
19801980
// If no filter is pushed down to Parquet, it should be the total length of data.
19811981
- assert(actual > 1 && actual < data.length)
1982-
+ // Only enable Comet test iff it's scan only, since with native execution
1982+
+ // Skip when Comet is enabled since with native execution
19831983
+ // `stripSparkFilter` can't remove the native filter
1984-
+ if (!isCometEnabled || isCometScanOnly) {
1984+
+ if (!isCometEnabled) {
19851985
+ assert(actual > 1 && actual < data.length)
19861986
+ }
19871987
}
@@ -1992,9 +1992,9 @@ index 8e88049f51e..20d7ef7b1bc 100644
19921992
// Note that, if record level filtering is enabled, it should be a single record.
19931993
// If no filter is pushed down to Parquet, it should be the total length of data.
19941994
- assert(actual > 1 && actual < data.length)
1995-
+ // Only enable Comet test iff it's scan only, since with native execution
1995+
+ // Skip when Comet is enabled since with native execution
19961996
+ // `stripSparkFilter` can't remove the native filter
1997-
+ if (!isCometEnabled || isCometScanOnly) {
1997+
+ if (!isCometEnabled) {
19981998
+ assert(actual > 1 && actual < data.length)
19991999
+ }
20002000
}
@@ -2878,32 +2878,14 @@ index e937173a590..7d20538bc68 100644
28782878
if (testTags.exists(_.isInstanceOf[DisableAdaptiveExecution])) {
28792879
super.test(testName, testTags: _*) {
28802880
withSQLConf(SQLConf.ADAPTIVE_EXECUTION_ENABLED.key -> "false") {
2881-
@@ -242,6 +272,29 @@ private[sql] trait SQLTestUtilsBase
2881+
@@ -242,6 +272,11 @@ private[sql] trait SQLTestUtilsBase
28822882
protected override def _sqlContext: SQLContext = self.spark.sqlContext
28832883
}
28842884

28852885
+ /**
28862886
+ * Whether Comet extension is enabled
28872887
+ */
28882888
+ protected def isCometEnabled: Boolean = SparkSession.isCometEnabled
2889-
+
2890-
+ /**
2891-
+ * Whether to enable ansi mode This is only effective when
2892-
+ * [[isCometEnabled]] returns true.
2893-
+ */
2894-
+ protected def enableCometAnsiMode: Boolean = {
2895-
+ val v = System.getenv("ENABLE_COMET_ANSI_MODE")
2896-
+ v != null && v.toBoolean
2897-
+ }
2898-
+
2899-
+ /**
2900-
+ * Whether Spark should only apply Comet scan optimization. This is only effective when
2901-
+ * [[isCometEnabled]] returns true.
2902-
+ */
2903-
+ protected def isCometScanOnly: Boolean = {
2904-
+ val v = System.getenv("ENABLE_COMET_SCAN_ONLY")
2905-
+ v != null && v.toBoolean
2906-
+ }
29072889
+
29082890
protected override def withSQLConf(pairs: (String, String)*)(f: => Unit): Unit = {
29092891
SparkSession.setActiveSession(spark)
@@ -2921,7 +2903,7 @@ diff --git a/sql/core/src/test/scala/org/apache/spark/sql/test/SharedSparkSessio
29212903
index ed2e309fa07..a5ea58146ad 100644
29222904
--- a/sql/core/src/test/scala/org/apache/spark/sql/test/SharedSparkSession.scala
29232905
+++ b/sql/core/src/test/scala/org/apache/spark/sql/test/SharedSparkSession.scala
2924-
@@ -74,6 +74,31 @@ trait SharedSparkSessionBase
2906+
@@ -74,6 +74,20 @@ trait SharedSparkSessionBase
29252907
// this rule may potentially block testing of other optimization rules such as
29262908
// ConstantPropagation etc.
29272909
.set(SQLConf.OPTIMIZER_EXCLUDED_RULES.key, ConvertToLocalRelation.ruleName)
@@ -2932,23 +2914,12 @@ index ed2e309fa07..a5ea58146ad 100644
29322914
+ .set("spark.comet.enabled", "true")
29332915
+ .set("spark.comet.parquet.respectFilterPushdown", "true")
29342916
+
2935-
+ if (!isCometScanOnly) {
2936-
+ conf
2937-
+ .set("spark.comet.exec.enabled", "true")
2938-
+ .set("spark.shuffle.manager",
2939-
+ "org.apache.spark.sql.comet.execution.shuffle.CometShuffleManager")
2940-
+ .set("spark.comet.exec.shuffle.enabled", "true")
2941-
+ .set("spark.comet.memoryOverhead", "10g")
2942-
+ } else {
2943-
+ conf
2944-
+ .set("spark.comet.exec.enabled", "false")
2945-
+ .set("spark.comet.exec.shuffle.enabled", "false")
2946-
+ }
2947-
+
2948-
+ if (enableCometAnsiMode) {
2949-
+ conf
2950-
+ .set("spark.sql.ansi.enabled", "true")
2951-
+ }
2917+
+ conf
2918+
+ .set("spark.comet.exec.enabled", "true")
2919+
+ .set("spark.shuffle.manager",
2920+
+ "org.apache.spark.sql.comet.execution.shuffle.CometShuffleManager")
2921+
+ .set("spark.comet.exec.shuffle.enabled", "true")
2922+
+ .set("spark.comet.memoryOverhead", "10g")
29522923
+ }
29532924
conf.set(
29542925
StaticSQLConf.WAREHOUSE_PATH,
@@ -3045,7 +3016,7 @@ diff --git a/sql/hive/src/test/scala/org/apache/spark/sql/hive/test/TestHive.sca
30453016
index 1d646f40b3e..5babe505301 100644
30463017
--- a/sql/hive/src/test/scala/org/apache/spark/sql/hive/test/TestHive.scala
30473018
+++ b/sql/hive/src/test/scala/org/apache/spark/sql/hive/test/TestHive.scala
3048-
@@ -53,25 +53,54 @@ object TestHive
3019+
@@ -53,25 +53,41 @@ object TestHive
30493020
new SparkContext(
30503021
System.getProperty("spark.sql.test.master", "local[1]"),
30513022
"TestSQLContext",
@@ -3092,24 +3063,11 @@ index 1d646f40b3e..5babe505301 100644
30923063
+ .set("spark.sql.extensions", "org.apache.comet.CometSparkSessionExtensions")
30933064
+ .set("spark.comet.enabled", "true")
30943065
+
3095-
+ val v = System.getenv("ENABLE_COMET_SCAN_ONLY")
3096-
+ if (v == null || !v.toBoolean) {
3097-
+ conf
3098-
+ .set("spark.comet.exec.enabled", "true")
3099-
+ .set("spark.shuffle.manager",
3100-
+ "org.apache.spark.sql.comet.execution.shuffle.CometShuffleManager")
3101-
+ .set("spark.comet.exec.shuffle.enabled", "true")
3102-
+ } else {
3103-
+ conf
3104-
+ .set("spark.comet.exec.enabled", "false")
3105-
+ .set("spark.comet.exec.shuffle.enabled", "false")
3106-
+ }
3107-
+
3108-
+ val a = System.getenv("ENABLE_COMET_ANSI_MODE")
3109-
+ if (a != null && a.toBoolean) {
3110-
+ conf
3111-
+ .set("spark.sql.ansi.enabled", "true")
3112-
+ }
3066+
+ conf
3067+
+ .set("spark.comet.exec.enabled", "true")
3068+
+ .set("spark.shuffle.manager",
3069+
+ "org.apache.spark.sql.comet.execution.shuffle.CometShuffleManager")
3070+
+ .set("spark.comet.exec.shuffle.enabled", "true")
31133071
+ }
31143072

31153073
+ conf

‎dev/diffs/4.0.2.diff‎

Lines changed: 18 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -2583,9 +2583,9 @@ index 6080a5e8e4b..ea058d57b4b 100644
25832583
// Note that, if record level filtering is enabled, it should be a single record.
25842584
// If no filter is pushed down to Parquet, it should be the total length of data.
25852585
- assert(actual > 1 && actual < data.length)
2586-
+ // Only enable Comet test iff it's scan only, since with native execution
2586+
+ // Skip when Comet is enabled since with native execution
25872587
+ // `stripSparkFilter` can't remove the native filter
2588-
+ if (!isCometEnabled || isCometScanOnly) {
2588+
+ if (!isCometEnabled) {
25892589
+ assert(actual > 1 && actual < data.length)
25902590
+ }
25912591
}
@@ -2596,9 +2596,9 @@ index 6080a5e8e4b..ea058d57b4b 100644
25962596
// Note that, if record level filtering is enabled, it should be a single record.
25972597
// If no filter is pushed down to Parquet, it should be the total length of data.
25982598
- assert(actual > 1 && actual < data.length)
2599-
+ // Only enable Comet test iff it's scan only, since with native execution
2599+
+ // Skip when Comet is enabled since with native execution
26002600
+ // `stripSparkFilter` can't remove the native filter
2601-
+ if (!isCometEnabled || isCometScanOnly) {
2601+
+ if (!isCometEnabled) {
26022602
+ assert(actual > 1 && actual < data.length)
26032603
+ }
26042604
}
@@ -3637,23 +3637,14 @@ index f0f3f94b811..f77b54dcef9 100644
36373637
if (testTags.exists(_.isInstanceOf[DisableAdaptiveExecution])) {
36383638
super.test(testName, testTags: _*) {
36393639
withSQLConf(SQLConf.ADAPTIVE_EXECUTION_ENABLED.key -> "false") {
3640-
@@ -248,8 +278,24 @@ private[sql] trait SQLTestUtilsBase
3640+
@@ -248,8 +278,15 @@ private[sql] trait SQLTestUtilsBase
36413641
override protected def converter: ColumnNodeToExpressionConverter = self.spark.converter
36423642
}
36433643

36443644
+ /**
36453645
+ * Whether Comet extension is enabled
36463646
+ */
36473647
+ protected def isCometEnabled: Boolean = SparkSession.isCometEnabled
3648-
+
3649-
+ /**
3650-
+ * Whether Spark should only apply Comet scan optimization. This is only effective when
3651-
+ * [[isCometEnabled]] returns true.
3652-
+ */
3653-
+ protected def isCometScanOnly: Boolean = {
3654-
+ val v = System.getenv("ENABLE_COMET_SCAN_ONLY")
3655-
+ v != null && v.toBoolean
3656-
+ }
36573648
+
36583649
protected override def withSQLConf[T](pairs: (String, String)*)(f: => T): T = {
36593650
SparkSession.setActiveSession(spark)
@@ -3675,7 +3666,7 @@ diff --git a/sql/core/src/test/scala/org/apache/spark/sql/test/SharedSparkSessio
36753666
index 245219c1756..a611836f086 100644
36763667
--- a/sql/core/src/test/scala/org/apache/spark/sql/test/SharedSparkSession.scala
36773668
+++ b/sql/core/src/test/scala/org/apache/spark/sql/test/SharedSparkSession.scala
3678-
@@ -75,6 +75,27 @@ trait SharedSparkSessionBase
3669+
@@ -75,6 +75,21 @@ trait SharedSparkSessionBase
36793670
// this rule may potentially block testing of other optimization rules such as
36803671
// ConstantPropagation etc.
36813672
.set(SQLConf.OPTIMIZER_EXCLUDED_RULES.key, ConvertToLocalRelation.ruleName)
@@ -3686,18 +3677,12 @@ index 245219c1756..a611836f086 100644
36863677
+ .set("spark.comet.enabled", "true")
36873678
+ .set("spark.comet.parquet.respectFilterPushdown", "true")
36883679
+
3689-
+ if (!isCometScanOnly) {
3690-
+ conf
3691-
+ .set("spark.comet.exec.enabled", "true")
3692-
+ .set("spark.shuffle.manager",
3693-
+ "org.apache.spark.sql.comet.execution.shuffle.CometShuffleManager")
3694-
+ .set("spark.comet.exec.shuffle.enabled", "true")
3695-
+ .set("spark.comet.memoryOverhead", "10g")
3696-
+ } else {
3697-
+ conf
3698-
+ .set("spark.comet.exec.enabled", "false")
3699-
+ .set("spark.comet.exec.shuffle.enabled", "false")
3700-
+ }
3680+
+ conf
3681+
+ .set("spark.comet.exec.enabled", "true")
3682+
+ .set("spark.shuffle.manager",
3683+
+ "org.apache.spark.sql.comet.execution.shuffle.CometShuffleManager")
3684+
+ .set("spark.comet.exec.shuffle.enabled", "true")
3685+
+ .set("spark.comet.memoryOverhead", "10g")
37013686
+
37023687
+ }
37033688
conf.set(
@@ -3824,7 +3809,7 @@ diff --git a/sql/hive/src/test/scala/org/apache/spark/sql/hive/test/TestHive.sca
38243809
index a394d0b7393..a4bc3d3fd8e 100644
38253810
--- a/sql/hive/src/test/scala/org/apache/spark/sql/hive/test/TestHive.scala
38263811
+++ b/sql/hive/src/test/scala/org/apache/spark/sql/hive/test/TestHive.scala
3827-
@@ -53,24 +53,41 @@ object TestHive
3812+
@@ -53,24 +53,34 @@ object TestHive
38283813
new SparkContext(
38293814
System.getProperty("spark.sql.test.master", "local[1]"),
38303815
"TestSQLContext",
@@ -3864,18 +3849,11 @@ index a394d0b7393..a4bc3d3fd8e 100644
38643849
+ .set("spark.sql.extensions", "org.apache.comet.CometSparkSessionExtensions")
38653850
+ .set("spark.comet.enabled", "true")
38663851
+
3867-
+ val v = System.getenv("ENABLE_COMET_SCAN_ONLY")
3868-
+ if (v == null || !v.toBoolean) {
3869-
+ conf
3870-
+ .set("spark.comet.exec.enabled", "true")
3871-
+ .set("spark.shuffle.manager",
3872-
+ "org.apache.spark.sql.comet.execution.shuffle.CometShuffleManager")
3873-
+ .set("spark.comet.exec.shuffle.enabled", "true")
3874-
+ } else {
3875-
+ conf
3876-
+ .set("spark.comet.exec.enabled", "false")
3877-
+ .set("spark.comet.exec.shuffle.enabled", "false")
3878-
+ }
3852+
+ conf
3853+
+ .set("spark.comet.exec.enabled", "true")
3854+
+ .set("spark.shuffle.manager",
3855+
+ "org.apache.spark.sql.comet.execution.shuffle.CometShuffleManager")
3856+
+ .set("spark.comet.exec.shuffle.enabled", "true")
38793857
+ }
38803858
+
38813859
+ conf

0 commit comments

Comments
 (0)