parthchandra commented on code in PR #2897:
URL: https://github.com/apache/datafusion-comet/pull/2897#discussion_r3034610341
##########
spark/src/main/scala/org/apache/comet/expressions/CometCast.scala:
##########
@@ -141,6 +141,11 @@ object CometCast extends CometExpressionSerde[Cast] with
CometExprShim {
(fromType, toType) match {
case (dt: ArrayType, _: ArrayType) if dt.elementType == NullType =>
Compatible()
+ case (ArrayType(DataTypes.DateType, _), ArrayType(toElementType, _))
+ if toElementType != DataTypes.StringType &&
Review Comment:
Shouldn't DateType -> IntegerType (and maybe others) be possible too?
##########
spark/src/test/scala/org/apache/comet/CometCastSuite.scala:
##########
@@ -1316,14 +1316,97 @@ class CometCastSuite extends CometTestBase with
AdaptiveSparkPlanHelper {
}
}
+ test("cast ArrayType to ArrayType") {
+ val types = Seq(
+ BooleanType,
+ StringType,
+ ByteType,
+ IntegerType,
+ LongType,
+ ShortType,
+ FloatType,
+ DoubleType,
+ DecimalType(10, 2),
+ DecimalType(38, 18),
+ DateType,
+ TimestampType,
+ BinaryType)
+ testArrayCastMatrix(types, ArrayType(_), generateArrays(100, _))
+ }
+
+ ignore("cast nested ArrayType to nested ArrayType") {
+ val types = Seq(
+ BooleanType,
+ StringType,
+ ByteType,
+ IntegerType,
+ LongType,
+ ShortType,
+ FloatType,
+ DoubleType,
+ DecimalType(10, 2),
+ DecimalType(38, 18),
+ DateType,
+ TimestampType,
+ BinaryType)
+ testArrayCastMatrix(
+ types,
+ dt => ArrayType(ArrayType(dt)),
+ dt => generateArrays(100, ArrayType(dt)))
+ }
+
+ private def testArrayCastMatrix(
+ elementTypes: Seq[DataType],
+ wrapType: DataType => DataType,
+ generateInput: DataType => DataFrame): Unit = {
+ for (fromType <- elementTypes) {
+ for (toType <- elementTypes) {
+ val name = s"cast $fromType to $toType"
+ val fromWrappedType = wrapType(fromType)
+ val toWrappedType = wrapType(toType)
+ if (fromType != toType &&
+ testNames.contains(name) &&
Review Comment:
nice !
##########
spark/src/test/scala/org/apache/comet/CometCastSuite.scala:
##########
@@ -1316,14 +1316,97 @@ class CometCastSuite extends CometTestBase with
AdaptiveSparkPlanHelper {
}
}
+ test("cast ArrayType to ArrayType") {
+ val types = Seq(
+ BooleanType,
+ StringType,
+ ByteType,
+ IntegerType,
+ LongType,
+ ShortType,
+ FloatType,
+ DoubleType,
+ DecimalType(10, 2),
+ DecimalType(38, 18),
+ DateType,
+ TimestampType,
+ BinaryType)
+ testArrayCastMatrix(types, ArrayType(_), generateArrays(100, _))
+ }
+
+ ignore("cast nested ArrayType to nested ArrayType") {
+ val types = Seq(
+ BooleanType,
+ StringType,
+ ByteType,
+ IntegerType,
+ LongType,
+ ShortType,
+ FloatType,
+ DoubleType,
+ DecimalType(10, 2),
+ DecimalType(38, 18),
+ DateType,
+ TimestampType,
+ BinaryType)
+ testArrayCastMatrix(
+ types,
+ dt => ArrayType(ArrayType(dt)),
+ dt => generateArrays(100, ArrayType(dt)))
+ }
+
+ private def testArrayCastMatrix(
+ elementTypes: Seq[DataType],
+ wrapType: DataType => DataType,
+ generateInput: DataType => DataFrame): Unit = {
+ for (fromType <- elementTypes) {
+ for (toType <- elementTypes) {
+ val name = s"cast $fromType to $toType"
+ val fromWrappedType = wrapType(fromType)
+ val toWrappedType = wrapType(toType)
+ if (fromType != toType &&
+ testNames.contains(name) &&
+ !tags
+ .get(name)
+ .exists(s => s.contains("org.scalatest.Ignore")) &&
+ Cast.canCast(fromWrappedType, toWrappedType) &&
+ CometCast.isSupported(fromWrappedType, toWrappedType, None,
CometEvalMode.LEGACY) ==
+ Compatible()) {
+ val legacyOnly =
+ fromType == DateType || (fromType == BooleanType && toType ==
TimestampType)
+ val ansiSupported =
+ CometCast.isSupported(fromWrappedType, toWrappedType, None,
CometEvalMode.ANSI) ==
+ Compatible()
+ val trySupported =
+ CometCast.isSupported(fromWrappedType, toWrappedType, None,
CometEvalMode.TRY) ==
+ Compatible()
+ castTest(
+ generateInput(fromType),
Review Comment:
minor: we can probably call `generateInput` once in the outer loop
##########
spark/src/test/scala/org/apache/comet/CometCastSuite.scala:
##########
@@ -1316,14 +1316,97 @@ class CometCastSuite extends CometTestBase with
AdaptiveSparkPlanHelper {
}
}
+ test("cast ArrayType to ArrayType") {
+ val types = Seq(
+ BooleanType,
+ StringType,
+ ByteType,
+ IntegerType,
+ LongType,
+ ShortType,
+ FloatType,
+ DoubleType,
+ DecimalType(10, 2),
+ DecimalType(38, 18),
+ DateType,
+ TimestampType,
+ BinaryType)
+ testArrayCastMatrix(types, ArrayType(_), generateArrays(100, _))
+ }
+
+ ignore("cast nested ArrayType to nested ArrayType") {
Review Comment:
Is there a github issue for this? Can we reference it here to document why
it is being ignored?
--
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]
