andygrove commented on code in PR #2176:
URL: https://github.com/apache/datafusion-comet/pull/2176#discussion_r2285405398


##########
spark/src/test/scala/org/apache/comet/parquet/ParquetReadSuite.scala:
##########
@@ -86,67 +86,74 @@ abstract class ParquetReadSuite extends CometTestBase {
   }
 
   test("unsupported Spark types") {
-    // for native iceberg compat, CometScanExec supports some types that 
native_comet does not.
-    // note that native_datafusion does not use CometScanExec so we need not 
include that in
-    // the check
-    val isDataFusionScan = usingDataSourceExec(conf)
-    Seq(
-      NullType -> false,
-      BooleanType -> true,
-      ByteType -> true,
-      ShortType -> true,
-      IntegerType -> true,
-      LongType -> true,
-      FloatType -> true,
-      DoubleType -> true,
-      BinaryType -> true,
-      StringType -> true,
-      // Timestamp here arbitrary for picking a concrete data type to from 
ArrayType
-      // Any other type works
-      ArrayType(TimestampType) -> isDataFusionScan,
-      StructType(
-        Seq(
-          StructField("f1", DecimalType.SYSTEM_DEFAULT),
-          StructField("f2", StringType))) -> isDataFusionScan,
-      MapType(keyType = LongType, valueType = DateType) -> isDataFusionScan,
-      StructType(
-        Seq(StructField("f1", ByteType), StructField("f2", StringType))) -> 
isDataFusionScan,
-      MapType(keyType = IntegerType, valueType = BinaryType) -> 
isDataFusionScan)
-      .foreach { case (dt, expected) =>
-        val fallbackReasons = new ListBuffer[String]()
-        assert(
-          CometScanTypeChecker(CometConf.COMET_NATIVE_SCAN_IMPL.get())
-            .isTypeSupported(dt, "", fallbackReasons) == expected)
-        // usingDataFusionParquetExec does not support CometBatchScanExec yet
-        if (!isDataFusionScan) {
-          assert(CometBatchScanExec.isTypeSupported(dt, "", fallbackReasons) 
== expected)
+    withSQLConf(CometConf.COMET_NATIVE_SCAN_IMPL.key -> 
CometConf.SCAN_NATIVE_COMET) {

Review Comment:
   I spent some time trying to get these two tests to pass but the 
implementations are not correct. I updated them to explicitly use 
`nstive_comet` for now (so no change in behavior) and filed a follow on issue 
to reimplement these two tests.
   
   https://github.com/apache/datafusion-comet/issues/2188
   
   We would also need to update or remove these tests as part of 
https://github.com/apache/datafusion-comet/issues/2177



-- 
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