viirya commented on code in PR #6826:
URL: https://github.com/apache/datafusion-comet/pull/6826#discussion_r4235982078
##########
spark/src/main/scala/org/apache/comet/expressions/CometCast.scala:
##########
@@ -119,10 +121,10 @@ object CometCast
inputs: Seq[Attribute],
binding: Boolean): Option[ExprOuterClass.Expr] = {
val cometEvalMode = evalMode(cast)
- cast.child match {
- case _: Literal =>
- exprToProtoInternal(Literal.create(cast.eval(), cast.dataType),
inputs, binding)
- case _ =>
+ foldLiteralCast(cast) match {
+ case Some(folded) =>
+ exprToProtoInternal(folded, inputs, binding)
+ case None =>
Review Comment:
Thanks. Another pre-existing native parsing bug: `parse_decimal_str`
computed the scale (fractional digits minus exponent) in i32, and
`parse_string_to_decimal` subtracted it from the target scale in i32, so both
wrapped in release builds. `java.math.BigDecimal` rejects a scale outside the
int range ("Scale out of range."), which Spark reports as `CAST_INVALID_INPUT`
under ANSI and NULL otherwise. The exponent itself was already parsed as i32,
which matches Java's "Exponent overflow." check.
Fixed in 21b7f9af8: the scale is computed in i64 and a scale that does not
fit in an int is invalid. Added a Rust unit test, reached/unreached conditional
cases for `'1e-2147483648'`, and a column `try_cast` case.
Since both findings were native parsers accepting input that Spark rejects,
I also compared `try_cast` from ~90 malformed strings to every string cast
target this path can reach (integral, floating, decimal, boolean, date,
timestamp, timestamp_ntz). It found no other input that Spark rejects and Comet
accepts.
--
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]
