Skip to content

Commit 9186e25

Browse files
committed
fix: make Unsupported require a reason string
Change Unsupported case class from Option[String] to String to prevent empty fallback reasons in explain output. This ensures every unsupported feature has a descriptive message explaining why it is not supported.
1 parent c3f59a6 commit 9186e25

17 files changed

Lines changed: 50 additions & 52 deletions

spark/src/main/scala/org/apache/comet/expressions/CometCast.scala

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -383,7 +383,7 @@ object CometCast extends CometExpressionSerde[Cast] with CometExprShim {
383383
DataTypes.IntegerType | DataTypes.LongType | DataTypes.BooleanType |
384384
DataTypes.TimestampType =>
385385
Compatible()
386-
case _ => Unsupported(Some(s"Cast from DecimalType to $toType is not supported"))
386+
case _ => Unsupported(s"Cast from DecimalType to $toType is not supported")
387387
}
388388

389389
private def canCastFromDate(toType: DataType, evalMode: CometEvalMode.Value): SupportLevel =
@@ -394,10 +394,10 @@ object CometCast extends CometExpressionSerde[Cast] with CometExprShim {
394394
DataTypes.IntegerType | DataTypes.LongType | DataTypes.FloatType |
395395
DataTypes.DoubleType | _: DecimalType if evalMode == CometEvalMode.LEGACY =>
396396
Compatible()
397-
case _ => Unsupported(Some(s"Cast from DateType to $toType is not supported"))
397+
case _ => Unsupported(s"Cast from DateType to $toType is not supported")
398398
}
399399

400400
private def unsupported(fromType: DataType, toType: DataType): Unsupported = {
401-
Unsupported(Some(s"Cast from $fromType to $toType is not supported"))
401+
Unsupported(s"Cast from $fromType to $toType is not supported")
402402
}
403403
}

spark/src/main/scala/org/apache/comet/rules/CometExecRule.scala

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -520,7 +520,7 @@ case class CometExecRule(session: SparkSession) extends Rule[SparkPlan] {
520520
if (handler.enabledConfig.forall(_.get(op.conf))) {
521521
handler.getSupportLevel(op) match {
522522
case Unsupported(notes) =>
523-
withInfo(op, notes.getOrElse(""))
523+
withInfo(op, notes)
524524
false
525525
case Incompatible(notes) =>
526526
val allowIncompat = CometConf.isOperatorAllowIncompat(opName)

spark/src/main/scala/org/apache/comet/serde/QueryPlanSerde.scala

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -482,7 +482,7 @@ object QueryPlanSerde extends Logging with CometExprShim {
482482
}
483483
aggHandler.getSupportLevel(fn) match {
484484
case Unsupported(notes) =>
485-
withInfo(fn, notes.getOrElse(""))
485+
withInfo(fn, notes)
486486
None
487487
case Incompatible(notes) =>
488488
val exprAllowIncompat = CometConf.isExprAllowIncompat(exprConfName)
@@ -609,7 +609,7 @@ object QueryPlanSerde extends Logging with CometExprShim {
609609
}
610610
handler.getSupportLevel(expr) match {
611611
case Unsupported(notes) =>
612-
withInfo(expr, notes.getOrElse(""))
612+
withInfo(expr, notes)
613613
None
614614
case Incompatible(notes) =>
615615
val exprAllowIncompat = CometConf.isExprAllowIncompat(exprConfName)

spark/src/main/scala/org/apache/comet/serde/SupportLevel.scala

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -39,4 +39,4 @@ case class Compatible(notes: Option[String] = None) extends SupportLevel
3939
case class Incompatible(notes: Option[String] = None) extends SupportLevel
4040

4141
/** Comet does not support this feature */
42-
case class Unsupported(notes: Option[String] = None) extends SupportLevel
42+
case class Unsupported(notes: String) extends SupportLevel

spark/src/main/scala/org/apache/comet/serde/arrays.scala

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -622,7 +622,8 @@ object CometArrayFilter extends CometExpressionSerde[ArrayFilter] {
622622
override def getSupportLevel(expr: ArrayFilter): SupportLevel = {
623623
expr.function.children.headOption match {
624624
case Some(_: IsNotNull) => Compatible()
625-
case _ => Unsupported()
625+
case _ =>
626+
Unsupported("Only array_compact (ArrayFilter with IsNotNull) is supported")
626627
}
627628
}
628629

@@ -639,10 +640,10 @@ object CometSize extends CometExpressionSerde[Size] {
639640
override def getSupportLevel(expr: Size): SupportLevel = {
640641
expr.child.dataType match {
641642
case _: ArrayType => Compatible()
642-
case _: MapType => Unsupported(Some("size does not support map inputs"))
643+
case _: MapType => Unsupported("size does not support map inputs")
643644
case other =>
644645
// this should be unreachable because Spark only supports map and array inputs
645-
Unsupported(Some(s"Unsupported child data type: $other"))
646+
Unsupported(s"Unsupported child data type: $other")
646647
}
647648
}
648649

spark/src/main/scala/org/apache/comet/serde/contraintExpressions.scala

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -33,8 +33,7 @@ object CometKnownFloatingPointNormalized
3333
case _: NormalizeNaNAndZero => Compatible()
3434
case _ =>
3535
Unsupported(
36-
Some(
37-
"KnownFloatingPointNormalized only supports NormalizeNaNAndZero child expressions"))
36+
"KnownFloatingPointNormalized only supports NormalizeNaNAndZero child expressions")
3837
}
3938
}
4039

spark/src/main/scala/org/apache/comet/serde/datetime.scala

Lines changed: 6 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -309,7 +309,7 @@ object CometUnixTimestamp extends CometExpressionSerde[UnixTimestamp] {
309309
Compatible()
310310
} else {
311311
val inputType = expr.children.head.dataType
312-
Unsupported(Some(s"unix_timestamp does not support input type: $inputType"))
312+
Unsupported(s"unix_timestamp does not support input type: $inputType")
313313
}
314314
}
315315

@@ -394,7 +394,7 @@ object CometTruncDate extends CometExpressionSerde[TruncDate] {
394394
if (supportedFormats.contains(fmt.toString.toLowerCase(Locale.ROOT))) {
395395
Compatible()
396396
} else {
397-
Unsupported(Some(s"Format $fmt is not supported"))
397+
Unsupported(s"Format $fmt is not supported")
398398
}
399399
case _ =>
400400
Incompatible(
@@ -454,7 +454,7 @@ object CometTruncTimestamp extends CometExpressionSerde[TruncTimestamp] {
454454
" (https://github.com/apache/datafusion-comet/issues/2649)"))
455455
}
456456
} else {
457-
Unsupported(Some(s"Format $fmt is not supported"))
457+
Unsupported(s"Format $fmt is not supported")
458458
}
459459
case _ =>
460460
Incompatible(
@@ -550,12 +550,11 @@ object CometDateFormat extends CometExpressionSerde[DateFormatClass] {
550550
}
551551
} else {
552552
Unsupported(
553-
Some(
554-
s"Format '$format' is not supported. Supported formats: " +
555-
supportedFormats.keys.mkString(", ")))
553+
s"Format '$format' is not supported. Supported formats: " +
554+
supportedFormats.keys.mkString(", "))
556555
}
557556
case _ =>
558-
Unsupported(Some("Only literal format strings are supported"))
557+
Unsupported("Only literal format strings are supported")
559558
}
560559
}
561560

spark/src/main/scala/org/apache/comet/serde/decimalExpressions.scala

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,7 @@ object CometMakeDecimal extends CometExpressionSerde[MakeDecimal] {
4242
override def getSupportLevel(expr: MakeDecimal): SupportLevel = {
4343
expr.child.dataType match {
4444
case LongType => Compatible()
45-
case other => Unsupported(Some(s"Unsupported input data type: $other"))
45+
case other => Unsupported(s"Unsupported input data type: $other")
4646
}
4747
}
4848

spark/src/main/scala/org/apache/comet/serde/literals.scala

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,7 @@ object CometLiteral extends CometExpressionSerde[Literal] with Logging {
5353
.isInstanceOf[ArrayType])))) {
5454
Compatible(None)
5555
} else {
56-
Unsupported(Some(s"Unsupported data type ${expr.dataType}"))
56+
Unsupported(s"Unsupported data type ${expr.dataType}")
5757
}
5858
}
5959

spark/src/main/scala/org/apache/comet/serde/math.scala

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -191,7 +191,7 @@ object CometAbs extends CometExpressionSerde[Abs] with MathExprBase {
191191
Compatible()
192192
case _ =>
193193
// Spark supports NumericType, DayTimeIntervalType, and YearMonthIntervalType
194-
Unsupported(Some("Only integral, floating-point, and decimal types are supported"))
194+
Unsupported("Only integral, floating-point, and decimal types are supported")
195195
}
196196
}
197197

@@ -244,7 +244,7 @@ object CometCheckOverflow extends CometExpressionSerde[CheckOverflow] {
244244
if (expr.dataType.isInstanceOf[DecimalType]) {
245245
Compatible()
246246
} else {
247-
Unsupported(Some("dataType must be DecimalType"))
247+
Unsupported("dataType must be DecimalType")
248248
}
249249
}
250250

0 commit comments

Comments
 (0)