sunchao commented on code in PR #5050:
URL: https://github.com/apache/datafusion-comet/pull/5050#discussion_r4125374680
##########
spark/src/main/scala/org/apache/comet/expressions/CometCast.scala:
##########
@@ -452,13 +471,19 @@ object CometCast
case _ => unsupported(DataTypes.DoubleType, toType)
}
- private def canCastFromDecimal(toType: DataType): SupportLevel = toType
match {
- case DataTypes.FloatType | DataTypes.DoubleType | DataTypes.ByteType |
DataTypes.ShortType |
- DataTypes.IntegerType | DataTypes.LongType | DataTypes.BooleanType |
- DataTypes.TimestampType =>
- Compatible()
- case _ => Unsupported(Some(s"Cast from DecimalType to $toType is not
supported"))
- }
+ private def canCastFromDecimal(fromType: DecimalType, toType: DataType):
SupportLevel =
+ toType match {
+ // Negative-scale source either overflows or loses precision natively;
see #5013.
+ case DataTypes.ByteType | DataTypes.ShortType | DataTypes.IntegerType |
DataTypes.LongType |
+ DataTypes.FloatType | DataTypes.DoubleType | DataTypes.TimestampType
+ if fromType.scale < 0 =>
+ Unsupported(Some(negativeScaleDecimalCastReason))
Review Comment:
[P2] Could you make the dispatcher’s negative-scale decimal input reader
safe before enabling this route? With `allowNegativeScaleOfDecimal=true`, a
materialized `Decimal(10,-1)` column containing `100`, `-100`, and `0` now
returns NULL for every `try_cast(v AS INT)`, instead of those integers. The
dispatcher selects `Decimal.createUnsafe` for precision <= 18. Spark’s integer
conversion then indexes `POW_10(-1)`, and TRY mode catches the resulting
exception and silently returns NULL. Before this guard, TRY casts used the
native Arrow path, which returns the correct values. The added tests collapse
the string-to-decimal construction into the dispatched expression and therefore
miss this input boundary. Use the BigDecimal-backed reader for negative scales,
or ensure genuine Spark fallback until that reader is safe.
Evidence: Reproduced twice on this exact head with a debug native build and
Spark 4.1.3/JDK 17. Enable `spark.sql.legacy.allowNegativeScaleOfDecimal=true`
and Comet local-table scans, and exclude `ConvertToLocalRelation`. Run
`Seq("100", "-100",
"0").toDF("s").select(col("s").cast(DecimalType(10,-1)).as("v"),
rand(123).as("r")).selectExpr("try_cast(v as int) as i", "r + r as r2")`. The
nondeterministic projection preserves the decimal input boundary. Spark returns
integer values 100, -100, and 0; Comet returns three NULLs. A second test
excluding `CollapseProject` reproduces this without `rand`. The unchanged
native TRY cast kernel separately returned `[Some(100), Some(-100), Some(0),
None]`. Sources and logs are preserved under `/tmp/review5050-final-cqri1bx4/`.
--
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]
