viirya commented on code in PR #6826:
URL: https://github.com/apache/datafusion-comet/pull/6826#discussion_r4235537471
##########
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, good catch. This is a pre-existing native parsing bug rather than
something specific to literals: `parse_string_to_float` stripped a trailing
d/D/f/F and handed the rest to Rust's parser, which accepts
`nan`/`inf`/`infinity` in any case. So a column hit it too, e.g. `try_cast(s AS
DOUBLE)` with `s = 'NaNF'` returned NaN where Spark returns NULL. Rust also
accepted `-nan`/`+nan`, which Java rejects (it only takes the exact-case
`+NaN`/`-NaN`).
Fixed in 17d33cab3: the suffix is now only allowed after a decimal number,
and a signed NaN only in Java's exact case. Added a Rust unit test whose
expectations come from `Double.parseDouble`/`Float.parseFloat` plus
`Cast.processFloatingPointSpecialLiterals`, the reached/unreached conditional
cases for `'NaNF'` and `'InfinityD'` in `cast_literal_ansi.sql`, and a column
`try_cast` case over these strings.
--
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]
