parthchandra commented on code in PR #3454:
URL: https://github.com/apache/datafusion-comet/pull/3454#discussion_r2914638784


##########
dev/diffs/4.0.1.diff:
##########
@@ -1,3 +1,43 @@
+diff --git 
a/core/src/test/scala/org/apache/spark/storage/FallbackStorageSuite.scala 
b/core/src/test/scala/org/apache/spark/storage/FallbackStorageSuite.scala
+index 6c51bd4ff2e..e72ec1d26e2 100644
+--- a/core/src/test/scala/org/apache/spark/storage/FallbackStorageSuite.scala
++++ b/core/src/test/scala/org/apache/spark/storage/FallbackStorageSuite.scala
+@@ -231,6 +231,11 @@ class FallbackStorageSuite extends SparkFunSuite with 
LocalSparkContext {
+   }
+ 
+   test("Upload from all decommissioned executors") {

Review Comment:
   IgnoreComet only works with tests extending `SQLTestUtils` (which overrides 
the `test` method). `FallbackStorageSuite` is a core module test and does not 
extendt `SQLTestUtils`.
   To override the test method in `SparkFunSuite` and also in `SQLTestUtils` 
one would end up probably creating a new mixin trait for `IgnoreComet` tests 
and I didn't want to complicate things with this PR. Also, it doesn't seem like 
we are likely to need it too often in core modules.



##########
dev/diffs/4.0.1.diff:
##########
@@ -3562,7 +3666,7 @@ index f0f3f94b811..d64e4e54e22 100644
 +   */
 +  protected def enableCometAnsiMode: Boolean = {
 +    val v = System.getenv("ENABLE_COMET_ANSI_MODE")
-+    v != null && v.toBoolean
++    if (v != null) v.toBoolean else true
 +  }

Review Comment:
   removed



-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to