findepi commented on code in PR #13806:
URL: https://github.com/apache/datafusion/pull/13806#discussion_r1889747569
##########
datafusion/sql/src/expr/value.rs:
##########
@@ -315,45 +321,84 @@ const fn try_decode_hex_char(c: u8) -> Option<u8> {
}
}
-/// Parse Decimal128 from a string
-///
-/// TODO: support parsing from scientific notation
-fn parse_decimal_128(unsigned_number: &str, negative: bool) -> Result<Expr> {
- // remove leading zeroes
- let trimmed = unsigned_number.trim_start_matches('0');
- // Parse precision and scale, remove decimal point if exists
- let (precision, scale, replaced_str) = if trimmed == "." {
- // Special cases for numbers such as “0.”, “000.”, and so on.
- (1, 0, Cow::Borrowed("0"))
- } else if let Some(i) = trimmed.find('.') {
- (
- trimmed.len() - 1,
- trimmed.len() - i - 1,
- Cow::Owned(trimmed.replace('.', "")),
- )
- } else {
- // No decimal point, keep as is
- (trimmed.len(), 0, Cow::Borrowed(trimmed))
- };
+/// Returns None if the value can't be converted to i256.
+/// Modified from
<https://github.com/apache/arrow-rs/blob/c4dbf0d8af6ca5a19b8b2ea777da3c276807fc5e/arrow-buffer/src/bigint/mod.rs#L303>
+fn bigint_to_i256(v: &BigInt) -> Option<i256> {
Review Comment:
please add unit tests for this function
it should be easy since both BigInt and i256 can convert to/from str
##########
datafusion/sql/src/expr/value.rs:
##########
@@ -315,45 +321,84 @@ const fn try_decode_hex_char(c: u8) -> Option<u8> {
}
}
-/// Parse Decimal128 from a string
-///
-/// TODO: support parsing from scientific notation
-fn parse_decimal_128(unsigned_number: &str, negative: bool) -> Result<Expr> {
- // remove leading zeroes
- let trimmed = unsigned_number.trim_start_matches('0');
- // Parse precision and scale, remove decimal point if exists
- let (precision, scale, replaced_str) = if trimmed == "." {
- // Special cases for numbers such as “0.”, “000.”, and so on.
- (1, 0, Cow::Borrowed("0"))
- } else if let Some(i) = trimmed.find('.') {
- (
- trimmed.len() - 1,
- trimmed.len() - i - 1,
- Cow::Owned(trimmed.replace('.', "")),
- )
- } else {
- // No decimal point, keep as is
- (trimmed.len(), 0, Cow::Borrowed(trimmed))
- };
+/// Returns None if the value can't be converted to i256.
+/// Modified from
<https://github.com/apache/arrow-rs/blob/c4dbf0d8af6ca5a19b8b2ea777da3c276807fc5e/arrow-buffer/src/bigint/mod.rs#L303>
+fn bigint_to_i256(v: &BigInt) -> Option<i256> {
+ let v_bytes = v.to_signed_bytes_le();
+ match v_bytes.len().cmp(&32) {
+ Ordering::Less => {
+ let mut bytes = if v.is_negative() {
+ [255_u8; 32]
+ } else {
+ [0; 32]
+ };
+ bytes[0..v_bytes.len()].copy_from_slice(&v_bytes[..v_bytes.len()]);
+ Some(i256::from_le_bytes(bytes))
+ }
+ Ordering::Equal =>
Some(i256::from_le_bytes(v_bytes.try_into().unwrap())),
+ Ordering::Greater => None,
+ }
+}
- let number = replaced_str.parse::<i128>().map_err(|e| {
+fn parse_decimal(unsigned_number: &str, negative: bool) -> Result<Expr> {
Review Comment:
Please add unit tests for this function, to exercise bound checks.
--
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: github-unsubscr...@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscr...@datafusion.apache.org
For additional commands, e-mail: github-h...@datafusion.apache.org
