feat: Implement Spark-compatible CAST float/double to string #346
Conversation
viirya
changed the title
viirya
changed the title
| 0.0, | ||
| -0.0) | ||
| ).toDF("a") | ||
| castTest(testValues, DataTypes.StringType, testAnsiModeThrows = false) |
There was a problem hiding this comment.
#351 will add some data generation function with random number. May need to rebase on it
There was a problem hiding this comment.
I will take a look
There was a problem hiding this comment.
If there are still some inputs that produce different results, we can document that in the compatibility guide and skip fuzz testing (or have some reduced form of fuzz testing). We could also consider adding a config so that the user can opt in or out of enabling this particular cast, depending on the severity of any incompatibilities.
There was a problem hiding this comment.
Also see #362 which improves how we handle casts that are not compatible with Spark
mattharder91
force-pushed
the
ISSUE-326/cast-float-string
branch
from
9f2dc41 to
f9e46c1
Compare
There was a problem hiding this comment.
Thans @mattharder91. I left some feedback. I think this is nearly ready to approve.
| @@ -329,9 +329,22 @@ class CometCastSuite extends CometTestBase with AdaptiveSparkPlanHelper { | |||
| castTest(generateFloats(), DataTypes.createDecimalType(10, 2)) | |||
| } | |||
|
|
|||
| ignore("cast FloatType to StringType") { | |||
| test("cast FloatType to StringType") { | |||
| // https://github.com/apache/datafusion-comet/issues/312 | |||
There was a problem hiding this comment.
We should remove the references to the issue now that we are resolving the issue
| // https://github.com/apache/datafusion-comet/issues/312 |
| @@ -374,9 +387,18 @@ class CometCastSuite extends CometTestBase with AdaptiveSparkPlanHelper { | |||
| castTest(generateDoubles(), DataTypes.createDecimalType(10, 2)) | |||
| } | |||
|
|
|||
| ignore("cast DoubleType to StringType") { | |||
| test("cast DoubleType to StringType") { | |||
| // https://github.com/apache/datafusion-comet/issues/312 | |||
There was a problem hiding this comment.
| // https://github.com/apache/datafusion-comet/issues/312 |
There was a problem hiding this comment.
We can remove these comments in a future PR
Which issue does this PR close?
Closes #312
Rationale for this change
Improve compatibility with Apache Spark
What changes are included in this PR?
How are these changes tested?
Further Considerations
A potential problem could be there for denormalized precision values.
Scalas
Double.MinPositiveValueis4.9E-324the denormalized value and parses correctly to4.9E-324while rust outputs5E-324.