You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@arrow.apache.org by GitBox <gi...@apache.org> on 2020/11/19 11:21:14 UTC

[GitHub] [arrow] Dandandan opened a new pull request #8714: ARROW-10654: Specialize parsers

Dandandan opened a new pull request #8714:
URL: https://github.com/apache/arrow/pull/8714


   Internal rust float parser is known to be slow.
   
   This change allows to have specialized implementations rather than relying on FromStr::parse.
   
   Also avoids calling `to_lowercase` for booleans.
   
   Would be nice to benchmark this.


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

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [arrow] Dandandan commented on pull request #8714: ARROW-10654: [Rust] Specialize parsing of floats / bools

Posted by GitBox <gi...@apache.org>.
Dandandan commented on pull request #8714:
URL: https://github.com/apache/arrow/pull/8714#issuecomment-730492620


   Did some benchmarking on this. Seems like a small win.
   
   Master:
   ```
   Running benchmarks with the following options: Opt { debug: false, iterations: 10, concurrency: 1, batch_size: 4096, path: "./yellow_tripdata_2020-01.csv", file_format: "csv" }
   
   Query 'fare_amt_by_passenger' iteration 0 took 4114 ms
   Query 'fare_amt_by_passenger' iteration 1 took 4087 ms
   Query 'fare_amt_by_passenger' iteration 2 took 4094 ms
   Query 'fare_amt_by_passenger' iteration 3 took 4118 ms
   Query 'fare_amt_by_passenger' iteration 4 took 4091 ms
   Query 'fare_amt_by_passenger' iteration 5 took 4099 ms
   Query 'fare_amt_by_passenger' iteration 6 took 4115 ms
   Query 'fare_amt_by_passenger' iteration 7 took 4129 ms
   Query 'fare_amt_by_passenger' iteration 8 took 4105 ms
   Query 'fare_amt_by_passenger' iteration 9 took 4095 ms
   ```
   
   This version:
   ```
   Running benchmarks with the following options: Opt { debug: false, iterations: 10, concurrency: 1, batch_size: 4096, path: "./yellow_tripdata_2020-01.csv", file_format: "csv" }
   Executing 'fare_amt_by_passenger'
   Query 'fare_amt_by_passenger' iteration 0 took 3985 ms
   Query 'fare_amt_by_passenger' iteration 1 took 3954 ms
   Query 'fare_amt_by_passenger' iteration 2 took 3961 ms
   Query 'fare_amt_by_passenger' iteration 3 took 3959 ms
   Query 'fare_amt_by_passenger' iteration 4 took 3963 ms
   Query 'fare_amt_by_passenger' iteration 5 took 3963 ms
   Query 'fare_amt_by_passenger' iteration 6 took 3958 ms
   Query 'fare_amt_by_passenger' iteration 7 took 3959 ms
   Query 'fare_amt_by_passenger' iteration 8 took 3971 ms
   Query 'fare_amt_by_passenger' iteration 9 took 3977 ms
   ```
   


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

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [arrow] alamb commented on a change in pull request #8714: ARROW-10654: [Rust] Specialize parsing of floats / bools in CSV Reader

Posted by GitBox <gi...@apache.org>.
alamb commented on a change in pull request #8714:
URL: https://github.com/apache/arrow/pull/8714#discussion_r528201732



##########
File path: rust/arrow/src/csv/reader.rs
##########
@@ -446,8 +446,57 @@ fn parse(
     arrays.and_then(|arr| RecordBatch::try_new(projected_schema, arr))
 }
 
+trait Parser: ArrowPrimitiveType {
+    fn parse(string: &str) -> Option<Self::Native> {
+        string.parse::<Self::Native>().ok()
+    }
+}
+
+impl Parser for BooleanType {
+    fn parse(string: &str) -> Option<bool> {

Review comment:
       I think the code in this PR is good as is -- I just think it would be nice to document what the behavior is more explicitly -- so I did  so in https://github.com/apache/arrow/pull/8733




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

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [arrow] Dandandan commented on a change in pull request #8714: ARROW-10654: [Rust] Specialize parsing of floats / bools

Posted by GitBox <gi...@apache.org>.
Dandandan commented on a change in pull request #8714:
URL: https://github.com/apache/arrow/pull/8714#discussion_r528190286



##########
File path: rust/arrow/src/csv/reader.rs
##########
@@ -446,8 +446,57 @@ fn parse(
     arrays.and_then(|arr| RecordBatch::try_new(projected_schema, arr))
 }
 
+trait Parser: ArrowPrimitiveType {
+    fn parse(string: &str) -> Option<Self::Native> {
+        string.parse::<Self::Native>().ok()
+    }
+}
+
+impl Parser for BooleanType {
+    fn parse(string: &str) -> Option<bool> {
+        if string.eq_ignore_ascii_case("false") {
+            return Some(false);
+        }
+        if string.eq_ignore_ascii_case("true") {
+            return Some(true);
+        }
+        None
+    }
+}
+
+impl Parser for Float32Type {
+    fn parse(string: &str) -> Option<f32> {
+        lexical_core::parse(string.as_bytes()).ok()
+    }
+}
+impl Parser for Float64Type {
+    fn parse(string: &str) -> Option<f64> {
+        lexical_core::parse(string.as_bytes()).ok()
+    }
+}
+
+impl Parser for UInt64Type {}

Review comment:
       Currently they are supported by passing a schema to the csv reader? And here they keep using the standard `string.parse::<T>` method.




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

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [arrow] alamb commented on a change in pull request #8714: ARROW-10654: [Rust] Specialize parsing of floats / bools in CSV Reader

Posted by GitBox <gi...@apache.org>.
alamb commented on a change in pull request #8714:
URL: https://github.com/apache/arrow/pull/8714#discussion_r528198753



##########
File path: rust/arrow/src/csv/reader.rs
##########
@@ -446,8 +446,57 @@ fn parse(
     arrays.and_then(|arr| RecordBatch::try_new(projected_schema, arr))
 }
 
+trait Parser: ArrowPrimitiveType {
+    fn parse(string: &str) -> Option<Self::Native> {
+        string.parse::<Self::Native>().ok()
+    }
+}
+
+impl Parser for BooleanType {
+    fn parse(string: &str) -> Option<bool> {

Review comment:
       I wondered how this related to the rust standard boolean parsing: https://doc.rust-lang.org/src/core/str/traits.rs.html#590
   
   Seems like it anything it would be slightly slower, but also support mixed case (`true` and `True`). Seems like a good improvement to me, though adding a test to encode the expected behavior would probably be a good idea.

##########
File path: rust/arrow/src/csv/reader.rs
##########
@@ -446,8 +446,57 @@ fn parse(
     arrays.and_then(|arr| RecordBatch::try_new(projected_schema, arr))
 }
 
+trait Parser: ArrowPrimitiveType {
+    fn parse(string: &str) -> Option<Self::Native> {
+        string.parse::<Self::Native>().ok()
+    }
+}
+
+impl Parser for BooleanType {
+    fn parse(string: &str) -> Option<bool> {
+        if string.eq_ignore_ascii_case("false") {
+            return Some(false);
+        }
+        if string.eq_ignore_ascii_case("true") {
+            return Some(true);
+        }
+        None
+    }
+}
+
+impl Parser for Float32Type {
+    fn parse(string: &str) -> Option<f32> {
+        lexical_core::parse(string.as_bytes()).ok()
+    }
+}
+impl Parser for Float64Type {
+    fn parse(string: &str) -> Option<f64> {
+        lexical_core::parse(string.as_bytes()).ok()
+    }
+}
+
+impl Parser for UInt64Type {}

Review comment:
       Perhaps @nevi-me  was asking if there are plans to improve the parsing performance of these types as well

##########
File path: rust/arrow/src/csv/reader.rs
##########
@@ -446,8 +446,57 @@ fn parse(
     arrays.and_then(|arr| RecordBatch::try_new(projected_schema, arr))
 }
 
+trait Parser: ArrowPrimitiveType {
+    fn parse(string: &str) -> Option<Self::Native> {
+        string.parse::<Self::Native>().ok()
+    }
+}
+
+impl Parser for BooleanType {
+    fn parse(string: &str) -> Option<bool> {

Review comment:
       Added https://issues.apache.org/jira/browse/ARROW-10677 to track extra parsing




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

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [arrow] Dandandan commented on pull request #8714: ARROW-10654: [Rust] Specialize parsing of floats / bools

Posted by GitBox <gi...@apache.org>.
Dandandan commented on pull request #8714:
URL: https://github.com/apache/arrow/pull/8714#issuecomment-730307198


   Some benchmark/context of string -> f64 is here https://github.com/Alexhuszagh/rust-lexical/


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

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [arrow] Dandandan commented on a change in pull request #8714: ARROW-10654: [Rust] Specialize parsing of floats / bools in CSV Reader

Posted by GitBox <gi...@apache.org>.
Dandandan commented on a change in pull request #8714:
URL: https://github.com/apache/arrow/pull/8714#discussion_r528201133



##########
File path: rust/arrow/src/csv/reader.rs
##########
@@ -446,8 +446,57 @@ fn parse(
     arrays.and_then(|arr| RecordBatch::try_new(projected_schema, arr))
 }
 
+trait Parser: ArrowPrimitiveType {
+    fn parse(string: &str) -> Option<Self::Native> {
+        string.parse::<Self::Native>().ok()
+    }
+}
+
+impl Parser for BooleanType {
+    fn parse(string: &str) -> Option<bool> {

Review comment:
       Yes, the mixed case is to keep compatibility with the previous implementation.
   
   Could as well have specific cases for all caps / capitalized booleans instead maybe? That would avoid accepting tRue etc.




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

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [arrow] alamb commented on a change in pull request #8714: ARROW-10654: [Rust] Specialize parsing of floats / bools in CSV Reader

Posted by GitBox <gi...@apache.org>.
alamb commented on a change in pull request #8714:
URL: https://github.com/apache/arrow/pull/8714#discussion_r528338450



##########
File path: rust/arrow/src/csv/reader.rs
##########
@@ -446,8 +446,57 @@ fn parse(
     arrays.and_then(|arr| RecordBatch::try_new(projected_schema, arr))
 }
 
+trait Parser: ArrowPrimitiveType {
+    fn parse(string: &str) -> Option<Self::Native> {
+        string.parse::<Self::Native>().ok()
+    }
+}
+
+impl Parser for BooleanType {
+    fn parse(string: &str) -> Option<bool> {
+        if string == "false" || string == "FALSE" || string == "False" {
+            return Some(true);

Review comment:
       it was my bad for not catching it in review as well




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

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [arrow] Dandandan edited a comment on pull request #8714: ARROW-10654: [Rust] Specialize parsing of floats / bools

Posted by GitBox <gi...@apache.org>.
Dandandan edited a comment on pull request #8714:
URL: https://github.com/apache/arrow/pull/8714#issuecomment-730307198


   Some benchmark/context of string -> f64 is here (note: log scale) https://github.com/Alexhuszagh/rust-lexical/


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

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [arrow] alamb closed pull request #8714: ARROW-10654: [Rust] Specialize parsing of floats / bools in CSV Reader

Posted by GitBox <gi...@apache.org>.
alamb closed pull request #8714:
URL: https://github.com/apache/arrow/pull/8714


   


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

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [arrow] github-actions[bot] commented on pull request #8714: ARROW-10654: [Rust] Specialize parsing of floats / bools

Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #8714:
URL: https://github.com/apache/arrow/pull/8714#issuecomment-730313603


   https://issues.apache.org/jira/browse/ARROW-10654


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

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [arrow] nevi-me commented on a change in pull request #8714: ARROW-10654: [Rust] Specialize parsing of floats / bools

Posted by GitBox <gi...@apache.org>.
nevi-me commented on a change in pull request #8714:
URL: https://github.com/apache/arrow/pull/8714#discussion_r528160949



##########
File path: rust/arrow/src/csv/reader.rs
##########
@@ -446,8 +446,57 @@ fn parse(
     arrays.and_then(|arr| RecordBatch::try_new(projected_schema, arr))
 }
 
+trait Parser: ArrowPrimitiveType {
+    fn parse(string: &str) -> Option<Self::Native> {
+        string.parse::<Self::Native>().ok()
+    }
+}
+
+impl Parser for BooleanType {
+    fn parse(string: &str) -> Option<bool> {
+        if string.eq_ignore_ascii_case("false") {
+            return Some(false);
+        }
+        if string.eq_ignore_ascii_case("true") {
+            return Some(true);
+        }
+        None
+    }
+}
+
+impl Parser for Float32Type {
+    fn parse(string: &str) -> Option<f32> {
+        lexical_core::parse(string.as_bytes()).ok()
+    }
+}
+impl Parser for Float64Type {
+    fn parse(string: &str) -> Option<f64> {
+        lexical_core::parse(string.as_bytes()).ok()
+    }
+}
+
+impl Parser for UInt64Type {}

Review comment:
       Would there be plans to support these at all?




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

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [arrow] Dandandan commented on a change in pull request #8714: ARROW-10654: [Rust] Specialize parsing of floats / bools in CSV Reader

Posted by GitBox <gi...@apache.org>.
Dandandan commented on a change in pull request #8714:
URL: https://github.com/apache/arrow/pull/8714#discussion_r528333021



##########
File path: rust/arrow/src/csv/reader.rs
##########
@@ -446,8 +446,57 @@ fn parse(
     arrays.and_then(|arr| RecordBatch::try_new(projected_schema, arr))
 }
 
+trait Parser: ArrowPrimitiveType {
+    fn parse(string: &str) -> Option<Self::Native> {
+        string.parse::<Self::Native>().ok()
+    }
+}
+
+impl Parser for BooleanType {
+    fn parse(string: &str) -> Option<bool> {
+        if string == "false" || string == "FALSE" || string == "False" {
+            return Some(true);

Review comment:
       O haha stupid bug, next time I'll make sure to add some unit tests directly in the PR.
   Thanks for fixing.




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

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [arrow] Dandandan commented on a change in pull request #8714: ARROW-10654: [Rust] Specialize parsing of floats / bools in CSV Reader

Posted by GitBox <gi...@apache.org>.
Dandandan commented on a change in pull request #8714:
URL: https://github.com/apache/arrow/pull/8714#discussion_r528201133



##########
File path: rust/arrow/src/csv/reader.rs
##########
@@ -446,8 +446,57 @@ fn parse(
     arrays.and_then(|arr| RecordBatch::try_new(projected_schema, arr))
 }
 
+trait Parser: ArrowPrimitiveType {
+    fn parse(string: &str) -> Option<Self::Native> {
+        string.parse::<Self::Native>().ok()
+    }
+}
+
+impl Parser for BooleanType {
+    fn parse(string: &str) -> Option<bool> {

Review comment:
       Yes, the mixed case is to keep compatibility with the previous implementation (it used to_lower)
   
   Could as well have specific cases for all caps / capitalized booleans instead maybe? That would avoid accepting tRue etc.




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

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [arrow] Dandandan commented on a change in pull request #8714: ARROW-10654: [Rust] Specialize parsing of floats / bools in CSV Reader

Posted by GitBox <gi...@apache.org>.
Dandandan commented on a change in pull request #8714:
URL: https://github.com/apache/arrow/pull/8714#discussion_r528201424



##########
File path: rust/arrow/src/csv/reader.rs
##########
@@ -446,8 +446,57 @@ fn parse(
     arrays.and_then(|arr| RecordBatch::try_new(projected_schema, arr))
 }
 
+trait Parser: ArrowPrimitiveType {
+    fn parse(string: &str) -> Option<Self::Native> {
+        string.parse::<Self::Native>().ok()
+    }
+}
+
+impl Parser for BooleanType {
+    fn parse(string: &str) -> Option<bool> {
+        if string.eq_ignore_ascii_case("false") {
+            return Some(false);
+        }
+        if string.eq_ignore_ascii_case("true") {
+            return Some(true);
+        }
+        None
+    }
+}
+
+impl Parser for Float32Type {
+    fn parse(string: &str) -> Option<f32> {
+        lexical_core::parse(string.as_bytes()).ok()
+    }
+}
+impl Parser for Float64Type {
+    fn parse(string: &str) -> Option<f64> {
+        lexical_core::parse(string.as_bytes()).ok()
+    }
+}
+
+impl Parser for UInt64Type {}

Review comment:
       Ah. It could special case on those as well. Difference between lexical core and standard lib don't seem too big here though




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

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [arrow] alamb commented on a change in pull request #8714: ARROW-10654: [Rust] Specialize parsing of floats / bools in CSV Reader

Posted by GitBox <gi...@apache.org>.
alamb commented on a change in pull request #8714:
URL: https://github.com/apache/arrow/pull/8714#discussion_r528200192



##########
File path: rust/arrow/src/csv/reader.rs
##########
@@ -446,8 +446,57 @@ fn parse(
     arrays.and_then(|arr| RecordBatch::try_new(projected_schema, arr))
 }
 
+trait Parser: ArrowPrimitiveType {
+    fn parse(string: &str) -> Option<Self::Native> {
+        string.parse::<Self::Native>().ok()
+    }
+}
+
+impl Parser for BooleanType {
+    fn parse(string: &str) -> Option<bool> {

Review comment:
       Added https://issues.apache.org/jira/browse/ARROW-10677 to track extra parsing 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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [arrow] alamb commented on pull request #8714: ARROW-10654: [Rust] Specialize parsing of floats / bools in CSV Reader

Posted by GitBox <gi...@apache.org>.
alamb commented on pull request #8714:
URL: https://github.com/apache/arrow/pull/8714#issuecomment-731650128


   CI is green, so merging


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

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [arrow] alamb commented on pull request #8714: ARROW-10654: [Rust] Specialize parsing of floats / bools in CSV Reader

Posted by GitBox <gi...@apache.org>.
alamb commented on pull request #8714:
URL: https://github.com/apache/arrow/pull/8714#issuecomment-731584471


   The CI failures don't seem related to this PR -- I am going to retrigger them


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

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [arrow] alamb commented on a change in pull request #8714: ARROW-10654: [Rust] Specialize parsing of floats / bools in CSV Reader

Posted by GitBox <gi...@apache.org>.
alamb commented on a change in pull request #8714:
URL: https://github.com/apache/arrow/pull/8714#discussion_r528328592



##########
File path: rust/arrow/src/csv/reader.rs
##########
@@ -446,8 +446,57 @@ fn parse(
     arrays.and_then(|arr| RecordBatch::try_new(projected_schema, arr))
 }
 
+trait Parser: ArrowPrimitiveType {
+    fn parse(string: &str) -> Option<Self::Native> {
+        string.parse::<Self::Native>().ok()
+    }
+}
+
+impl Parser for BooleanType {
+    fn parse(string: &str) -> Option<bool> {
+        if string == "false" || string == "FALSE" || string == "False" {
+            return Some(true);

Review comment:
       https://github.com/apache/arrow/pull/8733

##########
File path: rust/arrow/src/csv/reader.rs
##########
@@ -446,8 +446,57 @@ fn parse(
     arrays.and_then(|arr| RecordBatch::try_new(projected_schema, arr))
 }
 
+trait Parser: ArrowPrimitiveType {
+    fn parse(string: &str) -> Option<Self::Native> {
+        string.parse::<Self::Native>().ok()
+    }
+}
+
+impl Parser for BooleanType {
+    fn parse(string: &str) -> Option<bool> {
+        if string == "false" || string == "FALSE" || string == "False" {
+            return Some(true);

Review comment:
       I totally missed this, but this line has a bug parsing in this field appears to be backwards -- `"false"` seems to return `true` -- it looks like it got added in https://github.com/apache/arrow/pull/8714/commits/52bf0e839453749d1f042572a5e264b701e16904 -- FYI @Dandandan and @nevi-me -- I have a fix for it shortly




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

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [arrow] alamb commented on a change in pull request #8714: ARROW-10654: [Rust] Specialize parsing of floats / bools in CSV Reader

Posted by GitBox <gi...@apache.org>.
alamb commented on a change in pull request #8714:
URL: https://github.com/apache/arrow/pull/8714#discussion_r528201750



##########
File path: rust/arrow/src/csv/reader.rs
##########
@@ -446,8 +446,57 @@ fn parse(
     arrays.and_then(|arr| RecordBatch::try_new(projected_schema, arr))
 }
 
+trait Parser: ArrowPrimitiveType {
+    fn parse(string: &str) -> Option<Self::Native> {
+        string.parse::<Self::Native>().ok()
+    }
+}
+
+impl Parser for BooleanType {
+    fn parse(string: &str) -> Option<bool> {
+        if string.eq_ignore_ascii_case("false") {
+            return Some(false);
+        }
+        if string.eq_ignore_ascii_case("true") {
+            return Some(true);
+        }
+        None
+    }
+}
+
+impl Parser for Float32Type {
+    fn parse(string: &str) -> Option<f32> {
+        lexical_core::parse(string.as_bytes()).ok()
+    }
+}
+impl Parser for Float64Type {
+    fn parse(string: &str) -> Option<f64> {
+        lexical_core::parse(string.as_bytes()).ok()
+    }
+}
+
+impl Parser for UInt64Type {}

Review comment:
       yeah, I agree there is no reason to special case other types at this time




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

For queries about this service, please contact Infrastructure at:
users@infra.apache.org