Re: [PR] feat: Support casting string float types [datafusion-comet]

2025-12-23 Thread via GitHub


andygrove merged PR #2835:
URL: https://github.com/apache/datafusion-comet/pull/2835


-- 
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]



Re: [PR] feat: Support casting string float types [datafusion-comet]

2025-12-23 Thread via GitHub


coderfender commented on PR #2835:
URL: 
https://github.com/apache/datafusion-comet/pull/2835#issuecomment-3688312779

   @andygrove the CI is passing now . Please merge it whenever you get a chance 
. Thank you 


-- 
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]



Re: [PR] feat: Support casting string float types [datafusion-comet]

2025-12-23 Thread via GitHub


coderfender commented on PR #2835:
URL: 
https://github.com/apache/datafusion-comet/pull/2835#issuecomment-3688161015

   @andygrove  pushed a commit to fix clippy errors. Please kickoff CI  
whenever you get a chance 


-- 
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]



Re: [PR] feat: Support casting string float types [datafusion-comet]

2025-12-23 Thread via GitHub


coderfender commented on PR #2835:
URL: 
https://github.com/apache/datafusion-comet/pull/2835#issuecomment-3688138445

   @andygrove thank you very much . Let me fix the warning and push a commit 
right away 


-- 
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]



Re: [PR] feat: Support casting string float types [datafusion-comet]

2025-12-23 Thread via GitHub


andygrove commented on PR #2835:
URL: 
https://github.com/apache/datafusion-comet/pull/2835#issuecomment-3688136452

   @coderfender There is a clippy warning:
   
   ```
   Checking datafusion-comet-spark-expr v0.13.0 
(/__w/datafusion-comet/datafusion-comet/native/spark-expr)
   error: unused variable: `allow_incompat`
   --> spark-expr/src/conversion_funcs/cast.rs:1214:5
|
   1214 | allow_incompat: bool,
| ^^ help: if this is intentional, prefix it with an 
underscore: `_allow_incompat`
   ```


-- 
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]



Re: [PR] feat: Support casting string float types [datafusion-comet]

2025-12-23 Thread via GitHub


coderfender commented on PR #2835:
URL: 
https://github.com/apache/datafusion-comet/pull/2835#issuecomment-3687997086

   Thank you @andygrove 


-- 
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]



Re: [PR] feat: Support casting string float types [datafusion-comet]

2025-12-23 Thread via GitHub


coderfender commented on code in PR #2835:
URL: https://github.com/apache/datafusion-comet/pull/2835#discussion_r2644221243


##
native/spark-expr/src/conversion_funcs/cast.rs:
##
@@ -1190,11 +1262,13 @@ fn is_datafusion_spark_compatible(
 | DataType::Decimal256(_, _)
 | DataType::Utf8 // note that there can be formatting 
differences
 ),
-DataType::Utf8 if allow_incompat => matches!(
+DataType::Utf8 if allow_incompat => {
+matches!(to_type, DataType::Binary | DataType::Decimal128(_, _))

Review Comment:
   What are your thoughts ?
   



-- 
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]



Re: [PR] feat: Support casting string float types [datafusion-comet]

2025-12-23 Thread via GitHub


coderfender commented on code in PR #2835:
URL: https://github.com/apache/datafusion-comet/pull/2835#discussion_r2644220810


##
native/spark-expr/src/conversion_funcs/cast.rs:
##
@@ -1190,11 +1262,13 @@ fn is_datafusion_spark_compatible(
 | DataType::Decimal256(_, _)
 | DataType::Utf8 // note that there can be formatting 
differences
 ),
-DataType::Utf8 if allow_incompat => matches!(
+DataType::Utf8 if allow_incompat => {
+matches!(to_type, DataType::Binary | DataType::Decimal128(_, _))

Review Comment:
   @andygrove  thank you . Also I found that String -> Binary is marked 
compatible on the scala side but marked as `Incompatible` here . I will file an 
issue to fix that issue and fix it in a separate PR



-- 
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]



Re: [PR] feat: Support casting string float types [datafusion-comet]

2025-12-23 Thread via GitHub


coderfender commented on code in PR #2835:
URL: https://github.com/apache/datafusion-comet/pull/2835#discussion_r2643956880


##
spark/src/test/scala/org/apache/comet/CometCastSuite.scala:
##
@@ -642,34 +642,50 @@ class CometCastSuite extends CometTestBase with 
AdaptiveSparkPlanHelper {
 castTest(gen.generateStrings(dataSize, numericPattern, 8).toDF("a"), 
DataTypes.LongType)
   }
 
-  ignore("cast StringType to FloatType") {
+  test("cast StringType to DoubleType") {
 // https://github.com/apache/datafusion-comet/issues/326
-castTest(gen.generateStrings(dataSize, numericPattern, 8).toDF("a"), 
DataTypes.FloatType)
+castTest(gen.generateStrings(dataSize, numericPattern, 8).toDF("a"), 
DataTypes.DoubleType)
   }
 
-  test("cast StringType to FloatType (partial support)") {
-withSQLConf(
-  CometConf.getExprAllowIncompatConfigKey(classOf[Cast]) -> "true",
-  SQLConf.ANSI_ENABLED.key -> "false") {
-  castTest(
-gen.generateStrings(dataSize, "0123456789.", 8).toDF("a"),
-DataTypes.FloatType,
-testAnsi = false)
-}
+  test("cast StringType to FloatType") {
+castTest(gen.generateStrings(dataSize, numericPattern, 8).toDF("a"), 
DataTypes.DoubleType)

Review Comment:
   Thank you. I fixed the right data type 



-- 
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]



Re: [PR] feat: Support casting string float types [datafusion-comet]

2025-12-23 Thread via GitHub


andygrove commented on code in PR #2835:
URL: https://github.com/apache/datafusion-comet/pull/2835#discussion_r2643856374


##
spark/src/test/scala/org/apache/comet/CometCastSuite.scala:
##
@@ -642,34 +642,50 @@ class CometCastSuite extends CometTestBase with 
AdaptiveSparkPlanHelper {
 castTest(gen.generateStrings(dataSize, numericPattern, 8).toDF("a"), 
DataTypes.LongType)
   }
 
-  ignore("cast StringType to FloatType") {
+  test("cast StringType to DoubleType") {
 // https://github.com/apache/datafusion-comet/issues/326
-castTest(gen.generateStrings(dataSize, numericPattern, 8).toDF("a"), 
DataTypes.FloatType)
+castTest(gen.generateStrings(dataSize, numericPattern, 8).toDF("a"), 
DataTypes.DoubleType)
   }
 
-  test("cast StringType to FloatType (partial support)") {
-withSQLConf(
-  CometConf.getExprAllowIncompatConfigKey(classOf[Cast]) -> "true",
-  SQLConf.ANSI_ENABLED.key -> "false") {
-  castTest(
-gen.generateStrings(dataSize, "0123456789.", 8).toDF("a"),
-DataTypes.FloatType,
-testAnsi = false)
-}
+  test("cast StringType to FloatType") {
+castTest(gen.generateStrings(dataSize, numericPattern, 8).toDF("a"), 
DataTypes.DoubleType)

Review Comment:
   ```suggestion
   castTest(gen.generateStrings(dataSize, numericPattern, 8).toDF("a"), 
DataTypes.FloatType)
   ```



-- 
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]



Re: [PR] feat: Support casting string float types [datafusion-comet]

2025-12-23 Thread via GitHub


andygrove commented on code in PR #2835:
URL: https://github.com/apache/datafusion-comet/pull/2835#discussion_r2643854517


##
native/spark-expr/src/conversion_funcs/cast.rs:
##
@@ -1190,11 +1262,13 @@ fn is_datafusion_spark_compatible(
 | DataType::Decimal256(_, _)
 | DataType::Utf8 // note that there can be formatting 
differences
 ),
-DataType::Utf8 if allow_incompat => matches!(
+DataType::Utf8 if allow_incompat => {
+matches!(to_type, DataType::Binary | DataType::Decimal128(_, _))

Review Comment:
   I don't think `Decimal128` should be added here?



-- 
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]



Re: [PR] feat: Support casting string float types [datafusion-comet]

2025-12-22 Thread via GitHub


coderfender commented on code in PR #2835:
URL: https://github.com/apache/datafusion-comet/pull/2835#discussion_r2641717113


##
spark/src/test/scala/org/apache/comet/CometCastSuite.scala:
##
@@ -652,54 +678,61 @@ class CometCastSuite extends CometTestBase with 
AdaptiveSparkPlanHelper {
 castTest(gen.generateStrings(dataSize, numericPattern, 8).toDF("a"), 
DataTypes.LongType)
   }
 
-  ignore("cast StringType to FloatType") {
-// https://github.com/apache/datafusion-comet/issues/326
-castTest(gen.generateStrings(dataSize, numericPattern, 8).toDF("a"), 
DataTypes.FloatType)
-  }
-
-  test("cast StringType to FloatType (partial support)") {
-withSQLConf(
-  CometConf.getExprAllowIncompatConfigKey(classOf[Cast]) -> "true",
-  SQLConf.ANSI_ENABLED.key -> "false") {
-  castTest(
-gen.generateStrings(dataSize, "0123456789.", 8).toDF("a"),
-DataTypes.FloatType,
-testAnsi = false)
+  test("cast StringType to FloatType special values") {
+// TODO fix for Spark 4.0.0
+assume(!isSpark40Plus)
+Seq(true, false).foreach { v =>
+  castTest(specialValues.toDF("a"), DataTypes.FloatType, testAnsi = v)
 }
   }
 
-  ignore("cast StringType to DoubleType") {
-// https://github.com/apache/datafusion-comet/issues/326
-castTest(gen.generateStrings(dataSize, numericPattern, 8).toDF("a"), 
DataTypes.DoubleType)
+  test("cast StringType to DoubleType special values") {
+// TODO fix for Spark 4.0.0
+assume(!isSpark40Plus)
+Seq(true, false).foreach { v =>
+  castTest(specialValues.toDF("a"), DataTypes.DoubleType, testAnsi = v)
+}
   }
 
-  test("cast StringType to DoubleType (partial support)") {
-withSQLConf(
-  CometConf.getExprAllowIncompatConfigKey(classOf[Cast]) -> "true",
-  SQLConf.ANSI_ENABLED.key -> "false") {
+  test("cast StringType to DoubleType") {
+// TODO fix for Spark 4.0.0
+assume(!isSpark40Plus)
+Seq(true, false).foreach { v =>
   castTest(
-gen.generateStrings(dataSize, "0123456789.", 8).toDF("a"),
+gen.generateStrings(dataSize, numericPattern, 10).toDF("a"),
 DataTypes.DoubleType,
-testAnsi = false)
+testAnsi = v)
 }
   }
 
-  ignore("cast StringType to DecimalType(10,2)") {
-// https://github.com/apache/datafusion-comet/issues/325
-val values = gen.generateStrings(dataSize, numericPattern, 8).toDF("a")
-castTest(values, DataTypes.createDecimalType(10, 2))
+  test("cast StringType to FloatType") {
+// TODO fix for Spark 4.0.0
+assume(!isSpark40Plus)
+Seq(true, false).foreach { v =>
+  castTest(
+gen.generateStrings(dataSize, numericPattern, 10).toDF("a"),
+DataTypes.FloatType,
+testAnsi = v)
+}
   }
 
-  test("cast StringType to DecimalType(10,2) (partial support)") {
-withSQLConf(
-  CometConf.getExprAllowIncompatConfigKey(classOf[Cast]) -> "true",
-  SQLConf.ANSI_ENABLED.key -> "false") {
-  val values = gen
-.generateStrings(dataSize, "0123456789.", 8)
-.filter(_.exists(_.isDigit))
-.toDF("a")
-  castTest(values, DataTypes.createDecimalType(10, 2), testAnsi = false)
-}

Review Comment:
   Thank you for noticing it. These changes got removed while separating float 
and decimal casts 



-- 
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]



Re: [PR] feat: Support casting string float types [datafusion-comet]

2025-12-22 Thread via GitHub


coderfender commented on code in PR #2835:
URL: https://github.com/apache/datafusion-comet/pull/2835#discussion_r2641670977


##
spark/src/test/scala/org/apache/comet/CometCastSuite.scala:
##
@@ -652,54 +678,61 @@ class CometCastSuite extends CometTestBase with 
AdaptiveSparkPlanHelper {
 castTest(gen.generateStrings(dataSize, numericPattern, 8).toDF("a"), 
DataTypes.LongType)
   }
 
-  ignore("cast StringType to FloatType") {
-// https://github.com/apache/datafusion-comet/issues/326
-castTest(gen.generateStrings(dataSize, numericPattern, 8).toDF("a"), 
DataTypes.FloatType)
-  }
-
-  test("cast StringType to FloatType (partial support)") {
-withSQLConf(
-  CometConf.getExprAllowIncompatConfigKey(classOf[Cast]) -> "true",
-  SQLConf.ANSI_ENABLED.key -> "false") {
-  castTest(
-gen.generateStrings(dataSize, "0123456789.", 8).toDF("a"),
-DataTypes.FloatType,
-testAnsi = false)
+  test("cast StringType to FloatType special values") {
+// TODO fix for Spark 4.0.0
+assume(!isSpark40Plus)

Review Comment:
   We had an issue with Spark4x message matching but it got resolved with this 
PR : https://github.com/apache/datafusion-comet/pull/2919



##
native/spark-expr/src/conversion_funcs/cast.rs:
##
@@ -1058,6 +1053,84 @@ fn cast_array(
 Ok(spark_cast_postprocess(cast_result?, from_type, to_type))
 }
 
+fn cast_string_to_float(
+array: &ArrayRef,
+to_type: &DataType,
+eval_mode: EvalMode,
+) -> SparkResult {
+match to_type {
+DataType::Float16 | DataType::Float32 => {

Review Comment:
   Thank you . I fixed the code to not support `Float16` 



##
spark/src/test/scala/org/apache/comet/CometCastSuite.scala:
##
@@ -652,54 +678,61 @@ class CometCastSuite extends CometTestBase with 
AdaptiveSparkPlanHelper {
 castTest(gen.generateStrings(dataSize, numericPattern, 8).toDF("a"), 
DataTypes.LongType)
   }
 
-  ignore("cast StringType to FloatType") {
-// https://github.com/apache/datafusion-comet/issues/326
-castTest(gen.generateStrings(dataSize, numericPattern, 8).toDF("a"), 
DataTypes.FloatType)
-  }
-
-  test("cast StringType to FloatType (partial support)") {
-withSQLConf(
-  CometConf.getExprAllowIncompatConfigKey(classOf[Cast]) -> "true",
-  SQLConf.ANSI_ENABLED.key -> "false") {
-  castTest(
-gen.generateStrings(dataSize, "0123456789.", 8).toDF("a"),
-DataTypes.FloatType,
-testAnsi = false)
+  test("cast StringType to FloatType special values") {
+// TODO fix for Spark 4.0.0
+assume(!isSpark40Plus)
+Seq(true, false).foreach { v =>
+  castTest(specialValues.toDF("a"), DataTypes.FloatType, testAnsi = v)
 }
   }
 
-  ignore("cast StringType to DoubleType") {
-// https://github.com/apache/datafusion-comet/issues/326
-castTest(gen.generateStrings(dataSize, numericPattern, 8).toDF("a"), 
DataTypes.DoubleType)
+  test("cast StringType to DoubleType special values") {
+// TODO fix for Spark 4.0.0
+assume(!isSpark40Plus)
+Seq(true, false).foreach { v =>

Review Comment:
   Thank you . I changed the name of the running variable



-- 
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]



Re: [PR] feat: Support casting string float types [datafusion-comet]

2025-12-22 Thread via GitHub


andygrove commented on code in PR #2835:
URL: https://github.com/apache/datafusion-comet/pull/2835#discussion_r2640942135


##
spark/src/test/scala/org/apache/comet/CometCastSuite.scala:
##
@@ -652,54 +678,61 @@ class CometCastSuite extends CometTestBase with 
AdaptiveSparkPlanHelper {
 castTest(gen.generateStrings(dataSize, numericPattern, 8).toDF("a"), 
DataTypes.LongType)
   }
 
-  ignore("cast StringType to FloatType") {
-// https://github.com/apache/datafusion-comet/issues/326
-castTest(gen.generateStrings(dataSize, numericPattern, 8).toDF("a"), 
DataTypes.FloatType)
-  }
-
-  test("cast StringType to FloatType (partial support)") {
-withSQLConf(
-  CometConf.getExprAllowIncompatConfigKey(classOf[Cast]) -> "true",
-  SQLConf.ANSI_ENABLED.key -> "false") {
-  castTest(
-gen.generateStrings(dataSize, "0123456789.", 8).toDF("a"),
-DataTypes.FloatType,
-testAnsi = false)
+  test("cast StringType to FloatType special values") {
+// TODO fix for Spark 4.0.0
+assume(!isSpark40Plus)
+Seq(true, false).foreach { v =>
+  castTest(specialValues.toDF("a"), DataTypes.FloatType, testAnsi = v)
 }
   }
 
-  ignore("cast StringType to DoubleType") {
-// https://github.com/apache/datafusion-comet/issues/326
-castTest(gen.generateStrings(dataSize, numericPattern, 8).toDF("a"), 
DataTypes.DoubleType)
+  test("cast StringType to DoubleType special values") {
+// TODO fix for Spark 4.0.0
+assume(!isSpark40Plus)
+Seq(true, false).foreach { v =>

Review Comment:
   nit: `testAnsi` would be a more appopriate variable name than `v` for this 
and the following tests



-- 
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]



Re: [PR] feat: Support casting string float types [datafusion-comet]

2025-12-22 Thread via GitHub


andygrove commented on code in PR #2835:
URL: https://github.com/apache/datafusion-comet/pull/2835#discussion_r2640934603


##
spark/src/test/scala/org/apache/comet/CometCastSuite.scala:
##
@@ -652,54 +678,61 @@ class CometCastSuite extends CometTestBase with 
AdaptiveSparkPlanHelper {
 castTest(gen.generateStrings(dataSize, numericPattern, 8).toDF("a"), 
DataTypes.LongType)
   }
 
-  ignore("cast StringType to FloatType") {
-// https://github.com/apache/datafusion-comet/issues/326
-castTest(gen.generateStrings(dataSize, numericPattern, 8).toDF("a"), 
DataTypes.FloatType)
-  }
-
-  test("cast StringType to FloatType (partial support)") {
-withSQLConf(
-  CometConf.getExprAllowIncompatConfigKey(classOf[Cast]) -> "true",
-  SQLConf.ANSI_ENABLED.key -> "false") {
-  castTest(
-gen.generateStrings(dataSize, "0123456789.", 8).toDF("a"),
-DataTypes.FloatType,
-testAnsi = false)
+  test("cast StringType to FloatType special values") {
+// TODO fix for Spark 4.0.0
+assume(!isSpark40Plus)

Review Comment:
   What is the issue with Spark 4 support?



-- 
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]



Re: [PR] feat: Support casting string float types [datafusion-comet]

2025-12-22 Thread via GitHub


andygrove commented on code in PR #2835:
URL: https://github.com/apache/datafusion-comet/pull/2835#discussion_r2640909131


##
spark/src/test/scala/org/apache/comet/CometCastSuite.scala:
##
@@ -652,54 +678,61 @@ class CometCastSuite extends CometTestBase with 
AdaptiveSparkPlanHelper {
 castTest(gen.generateStrings(dataSize, numericPattern, 8).toDF("a"), 
DataTypes.LongType)
   }
 
-  ignore("cast StringType to FloatType") {
-// https://github.com/apache/datafusion-comet/issues/326
-castTest(gen.generateStrings(dataSize, numericPattern, 8).toDF("a"), 
DataTypes.FloatType)
-  }
-
-  test("cast StringType to FloatType (partial support)") {
-withSQLConf(
-  CometConf.getExprAllowIncompatConfigKey(classOf[Cast]) -> "true",
-  SQLConf.ANSI_ENABLED.key -> "false") {
-  castTest(
-gen.generateStrings(dataSize, "0123456789.", 8).toDF("a"),
-DataTypes.FloatType,
-testAnsi = false)
+  test("cast StringType to FloatType special values") {
+// TODO fix for Spark 4.0.0
+assume(!isSpark40Plus)
+Seq(true, false).foreach { v =>
+  castTest(specialValues.toDF("a"), DataTypes.FloatType, testAnsi = v)
 }
   }
 
-  ignore("cast StringType to DoubleType") {
-// https://github.com/apache/datafusion-comet/issues/326
-castTest(gen.generateStrings(dataSize, numericPattern, 8).toDF("a"), 
DataTypes.DoubleType)
+  test("cast StringType to DoubleType special values") {
+// TODO fix for Spark 4.0.0
+assume(!isSpark40Plus)
+Seq(true, false).foreach { v =>
+  castTest(specialValues.toDF("a"), DataTypes.DoubleType, testAnsi = v)
+}
   }
 
-  test("cast StringType to DoubleType (partial support)") {
-withSQLConf(
-  CometConf.getExprAllowIncompatConfigKey(classOf[Cast]) -> "true",
-  SQLConf.ANSI_ENABLED.key -> "false") {
+  test("cast StringType to DoubleType") {
+// TODO fix for Spark 4.0.0
+assume(!isSpark40Plus)
+Seq(true, false).foreach { v =>
   castTest(
-gen.generateStrings(dataSize, "0123456789.", 8).toDF("a"),
+gen.generateStrings(dataSize, numericPattern, 10).toDF("a"),
 DataTypes.DoubleType,
-testAnsi = false)
+testAnsi = v)
 }
   }
 
-  ignore("cast StringType to DecimalType(10,2)") {
-// https://github.com/apache/datafusion-comet/issues/325
-val values = gen.generateStrings(dataSize, numericPattern, 8).toDF("a")
-castTest(values, DataTypes.createDecimalType(10, 2))
+  test("cast StringType to FloatType") {
+// TODO fix for Spark 4.0.0
+assume(!isSpark40Plus)
+Seq(true, false).foreach { v =>
+  castTest(
+gen.generateStrings(dataSize, numericPattern, 10).toDF("a"),
+DataTypes.FloatType,
+testAnsi = v)
+}
   }
 
-  test("cast StringType to DecimalType(10,2) (partial support)") {
-withSQLConf(
-  CometConf.getExprAllowIncompatConfigKey(classOf[Cast]) -> "true",
-  SQLConf.ANSI_ENABLED.key -> "false") {
-  val values = gen
-.generateStrings(dataSize, "0123456789.", 8)
-.filter(_.exists(_.isDigit))
-.toDF("a")
-  castTest(values, DataTypes.createDecimalType(10, 2), testAnsi = false)
-}

Review Comment:
   The string to decimal tests should not be removed



-- 
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]



Re: [PR] feat: Support casting string float types [datafusion-comet]

2025-12-22 Thread via GitHub


andygrove commented on code in PR #2835:
URL: https://github.com/apache/datafusion-comet/pull/2835#discussion_r2640880615


##
native/spark-expr/src/conversion_funcs/cast.rs:
##
@@ -1058,6 +1053,84 @@ fn cast_array(
 Ok(spark_cast_postprocess(cast_result?, from_type, to_type))
 }
 
+fn cast_string_to_float(
+array: &ArrayRef,
+to_type: &DataType,
+eval_mode: EvalMode,
+) -> SparkResult {
+match to_type {
+DataType::Float16 | DataType::Float32 => {

Review Comment:
   I don't think we should handle `Float16` since Spark does not support that 
type



-- 
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]