alamb commented on code in PR #8700:
URL: https://github.com/apache/arrow-rs/pull/8700#discussion_r2692296831


##########
arrow-cast/src/parse.rs:
##########
@@ -850,7 +835,9 @@ fn parse_e_notation<T: DecimalType>(
 }
 
 /// Parse the string format decimal value to i128/i256 format and checking the 
precision and scale.
-/// The result value can't be out of bounds.
+/// Expected behavior:
+/// - the result value can't be out of bounds.
+/// - when parsing a decimal with scale 0, any digits pass the decimal point 
will be discarded

Review Comment:
   I think for e-notation the digits after the decimal do affect the value 
(e.g., "1.23e4" → 12300) as shown in the tests. Can we clarify this slightly?



##########
arrow-cast/src/parse.rs:
##########
@@ -2752,6 +2740,35 @@ mod tests {
             let result = parse_decimal::<Decimal256Type>(s, 76, scale);
             assert_eq!(i, result.unwrap());
         }
+
+        let zero_scale_tests = [
+            ("0.123", 0, 3),
+            ("1.0", 1, 3),
+            ("1.2", 1, 3),
+            ("1.00", 1, 3),
+            ("1.23", 1, 3),
+            ("1.000", 1, 3),
+            ("1.123", 1, 3),
+            ("123.0", 123, 3),
+            ("123.4", 123, 3),
+            ("123.00", 123, 3),
+            ("123.45", 123, 3),
+            ("123.000000000000000000004", 123, 3),
+            ("0.123e2", 12, 3),
+            ("0.123e4", 1230, 10),
+            ("1.23e4", 12300, 10),
+            ("12.3e4", 123000, 10),
+            ("123e4", 1230000, 10),
+            (
+                "20000000000000000000000000000000000002.0",
+                20000000000000000000000000000000000002,
+                38,
+            ),
+        ];
+        for (s, i, precision) in zero_scale_tests {
+            let result_128 = parse_decimal::<Decimal128Type>(s, precision, 
0).unwrap();
+            assert_eq!(i, result_128);
+        }

Review Comment:
   Could you also add some error tests, for example
   -  `.` 
   - `blag`
   
   Should both fail to parse



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

Reply via email to