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