Skip to content

Commit 577a793

Browse files
authored
fix: clean up CometCast support-level reporting (apache#4501) (apache#4595)
1 parent 107a611 commit 577a793

2 files changed

Lines changed: 16 additions & 14 deletions

File tree

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

Lines changed: 14 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,10 @@ import org.apache.comet.shims.CometExprShim
3232

3333
object CometCast extends CometExpressionSerde[Cast] with CometExprShim {
3434

35+
// Shared with CometCastSuite so the asserted reason cannot drift from production.
36+
private[comet] val negativeScaleDecimalToStringReason: String =
37+
"Negative-scale decimal requires spark.sql.legacy.allowNegativeScaleOfDecimal=true"
38+
3539
def supportedTypes: Seq[DataType] =
3640
Seq(
3741
DataTypes.BooleanType,
@@ -48,17 +52,17 @@ object CometCast extends CometExpressionSerde[Cast] with CometExprShim {
4852
DataTypes.TimestampType,
4953
DataTypes.TimestampNTZType)
5054

51-
override def getIncompatibleReasons(): Seq[String] = Seq(
52-
"Some cast operations between specific type pairs may produce different results than Spark." +
53-
" Refer to the compatibility guide for the full matrix of supported cast operations.")
54-
55-
override def getUnsupportedReasons(): Seq[String] = Seq(
56-
"Not all cast type combinations are supported. Unsupported casts fall back to Spark.")
55+
// Note: `getIncompatibleReasons` / `getUnsupportedReasons` are intentionally not
56+
// overridden here. The per-pair `isSupported` matrix is the canonical source of cast
57+
// reasons: `cast.md` is generated directly from it (see `GenerateDocs.writeCastMatrixForMode`)
58+
// and EXPLAIN surfaces the per-pair reason via `getSupportLevel`. A single static sentence
59+
// would only duplicate the matrix and risk drifting from it.
5760

5861
override def getSupportLevel(cast: Cast): SupportLevel = {
5962
if (cast.child.isInstanceOf[Literal]) {
60-
// casting from literal is compatible because we delegate to Spark
61-
// further data type checks will be performed by CometLiteral
63+
// A cast whose child is a literal is folded by Spark at planning time via `cast.eval()`
64+
// (see `convert`), so the cast never executes natively and the result matches Spark by
65+
// definition. `CometLiteral` then validates the resulting literal's data type.
6266
Compatible()
6367
} else {
6468
isSupported(cast.child.dataType, cast.dataType, cast.timeZoneId, evalMode(cast))
@@ -254,7 +258,8 @@ object CometCast extends CometExpressionSerde[Cast] with CometExprShim {
254258
val allowNegativeScale = SQLConf.get
255259
.getConfString("spark.sql.legacy.allowNegativeScaleOfDecimal", "false")
256260
.toBoolean
257-
if (allowNegativeScale) Compatible() else Incompatible()
261+
if (allowNegativeScale) Compatible()
262+
else Incompatible(Some(negativeScaleDecimalToStringReason))
258263
case _: DecimalType =>
259264
// Compatible across all eval modes: LEGACY uses cast_decimal128_to_utf8 which
260265
// replicates Java BigDecimal.toString() (scientific notation when adj_exp < -6);

spark/src/test/scala/org/apache/comet/CometCastSuite.scala

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -787,11 +787,8 @@ class CometCastSuite extends CometTestBase with AdaptiveSparkPlanHelper {
787787
}
788788
withSQLConf("spark.sql.legacy.allowNegativeScaleOfDecimal" -> "false") {
789789
assert(
790-
CometCast.isSupported(
791-
negScaleType,
792-
DataTypes.StringType,
793-
None,
794-
CometEvalMode.LEGACY) == Incompatible())
790+
CometCast.isSupported(negScaleType, DataTypes.StringType, None, CometEvalMode.LEGACY) ==
791+
Incompatible(Some(CometCast.negativeScaleDecimalToStringReason)))
795792
}
796793
}
797794

0 commit comments

Comments
 (0)