gruuya commented on code in PR #8700:
URL: https://github.com/apache/arrow-rs/pull/8700#discussion_r2689730978
##########
arrow-cast/src/parse.rs:
##########
@@ -741,72 +741,53 @@ fn parse_e_notation<T: DecimalType>(
let mut exp: i16 = 0;
let base = T::Native::usize_as(10);
- let mut exp_start: bool = false;
// e has a plus sign
let mut pos_shift_direction: bool = true;
- // skip to point or exponent index
- let mut bs;
- if fractionals > 0 {
- // it's a fraction, so the point index needs to be skipped, so +1
- bs = s.as_bytes().iter().skip(index + fractionals as usize + 1);
- } else {
- // it's actually an integer that is already written into the result,
so let's skip on to e
- bs = s.as_bytes().iter().skip(index);
- }
+ // skip to the exponent index directly or just after any processed
fractionals
+ let mut bs = s.as_bytes().iter().skip(index + fractionals as usize);
Review Comment:
That makes sense, though this leads to some cascading casting and type
changes, as we use `fractionals` to compute/compare with other variables,
notably with `scale` which is `i8`.
So we can then make `scale` an unsigned type too, since it looks like
negative scale is not supported right now.
However I'd rather retain the possibility of supporting negative scales at
some point, so I went with changing the `scale` type to `i16` instead. That way
`fractionals: u8` can be safely cast to `i16` for comparison/arithmetic (plus
`parse_e_notation` already expects `scale` to be `i16`).
--
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]
