Skip to content
93 changes: 66 additions & 27 deletions arrow-cast/src/parse.rs
Original file line number Diff line number Diff line change
Expand Up @@ -445,7 +445,10 @@ pub trait Parser: ArrowPrimitiveType {

impl Parser for Float16Type {
fn parse(string: &str) -> Option<f16> {
let string = truncate_white_space(string);
if let Ok(raw_float) = lexical_core::parse(string.as_bytes()) {
return Some(f16::from_f32(raw_float));
}
let string = trim_pre_and_post_whitespace(string);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is a good strategy -- try the common/ fast path first and then fallback to checking whitespace afterwards

lexical_core::parse(string.as_bytes())
.ok()
.map(f16::from_f32)
Expand All @@ -454,49 +457,59 @@ impl Parser for Float16Type {

impl Parser for Float32Type {
fn parse(string: &str) -> Option<f32> {
let string = truncate_white_space(string);
if let Ok(raw_float) = lexical_core::parse(string.as_bytes()) {
return Some(raw_float);
}
let string = trim_pre_and_post_whitespace(string);
lexical_core::parse(string.as_bytes()).ok()
}
}

impl Parser for Float64Type {
fn parse(string: &str) -> Option<f64> {
let string = truncate_white_space(string);
if let Ok(raw_float) = lexical_core::parse(string.as_bytes()) {
return Some(raw_float);
}
let string = trim_pre_and_post_whitespace(string);
Comment thread
Rich-T-kid marked this conversation as resolved.
lexical_core::parse(string.as_bytes()).ok()
}
}

/// this is a no-op if the string starts and ends with a digit, otherwise it will trim whitespace from the start and end of the string.
#[inline]
fn truncate_white_space(string: &str) -> &str {
if string
.as_bytes()
.first()
.is_some_and(|first_byte| !first_byte.is_ascii_digit())
{
string.trim_ascii_start()
} else {
string
fn trim_pre_and_post_whitespace(string: &str) -> &str {
let bytes = string.as_bytes();
let prefix = bytes.first().is_some_and(|b| !b.is_ascii_digit());
let suffix = bytes.last().is_some_and(|b| !b.is_ascii_digit());
match (prefix, suffix) {
(false, false) => string,
(true, false) => string.trim_ascii_start(),
(false, true) => string.trim_ascii_end(),
(true, true) => string.trim_ascii(),
}
}

macro_rules! parser_primitive {
($t:ty) => {
impl Parser for $t {
fn parse(mut string: &str) -> Option<Self::Native> {
if !string.as_bytes().last().is_some_and(|x| x.is_ascii_digit()) {
return None;
fn parse(string: &str) -> Option<Self::Native> {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

perhaps we can add one benchmark for i32 just so we have a reference

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

added those here #10421

let mut raw_bytes = string.as_bytes();
if !raw_bytes.last().is_some_and(|x| x.is_ascii_digit()) {
raw_bytes = raw_bytes.trim_ascii_end();
if !raw_bytes.last().is_some_and(|x| x.is_ascii_digit()) {
return None;
}
}
if string
.as_bytes()
.first()
.is_some_and(|first| !first.is_ascii_digit())
{
string = string.trim_ascii_start();
};
match atoi::FromRadix10SignedChecked::from_radix_10_signed_checked(
string.as_bytes(),
) {
(Some(n), x) if x == string.len() => Some(n),
_ => None,
match atoi::FromRadix10SignedChecked::from_radix_10_signed_checked(raw_bytes) {
(Some(n), x) if x == raw_bytes.len() => Some(n),
_ => {
let trimmed = raw_bytes.trim_ascii_start();
match atoi::FromRadix10SignedChecked::from_radix_10_signed_checked(trimmed)
{
(Some(n), x) if x == trimmed.len() => Some(n),
_ => None,
}
}
}
}
}
Expand Down Expand Up @@ -2913,5 +2926,31 @@ mod tests {
assert_eq!(Int32Type::parse("\t\n\t\n\n\n\t1"), Some(1));
assert_eq!(Int32Type::parse(" \n-25"), Some(-25));
assert_eq!(Int32Type::parse("\t-800"), Some(-800));

// suffix whitespace
assert_eq!(Float64Type::parse("1.5 "), Some(1.5));
assert_eq!(Float64Type::parse("40.5123\n"), Some(40.5123));
assert_eq!(Float64Type::parse("40.5123\n\t\n\t\n"), Some(40.5123));
assert_eq!(Float64Type::parse("-942.5423\t"), Some(-942.5423));
assert_eq!(Int32Type::parse("3 "), Some(3));
assert_eq!(Int32Type::parse("30 "), Some(30));
assert_eq!(Int32Type::parse("-25 \n"), Some(-25));
assert_eq!(Int32Type::parse("800\t"), Some(800));
// whitespace on both sides
assert_eq!(Float64Type::parse(" 1.5 "), Some(1.5));
assert_eq!(Float64Type::parse("\t\n 20.54 \t"), Some(20.54));
assert_eq!(Float64Type::parse("\n-942.5423\n"), Some(-942.5423));
assert_eq!(Int32Type::parse(" 3 "), Some(3));
assert_eq!(Int32Type::parse("\n \n 100 \n"), Some(100));
assert_eq!(Int32Type::parse("\t-800\t\n"), Some(-800));

// trailing non-whitespace chars should not parse
assert_eq!(Float64Type::parse("1.5abc"), None);
assert_eq!(Float64Type::parse("40.5123x"), None);
assert_eq!(Int32Type::parse("30x"), None);
assert_eq!(Int32Type::parse("100px"), None);
assert_eq!(Int32Type::parse("-25!"), None);
assert_eq!(Int32Type::parse("3j"), None);
assert_eq!(Int32Type::parse("3"), Some(3));
}
}
Loading