ARROW-11123: [Rust] Use cast kernel to simplify csv parser

[Dandandan]

This came up in #9084 when discussing the PR with @jorgecarleitao

One of the ideas here is that we could take advantage of the cast kernel in Arrow to parse the csv column, deduplicating code and making more use of Arrow. We can get rid of the custom parsers and parsing / building primitive arrays.

On loading the CSV in the TCPH benchmark looks like this has a small negative effect on performance.

[github-actions]

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

[andygrove]

I like the idea of using CAST here for consistency.

[Dandandan]

I think this needs to be done to fully implement this PR:

• Implement cast that returns an error on parsing errors instead of null
• Implement cast(utf8, bool) to remove remaining parser function
• Some (cast) / stringarray ( ARROW-11037: [Rust] Optimized creation of string array from iterator. #9016 ) perf. improvements to avoid performance hit

I think an eventual performance benefit is that it makes it easier to get rid of the (slow) StringRecord eventually as we can just load bytes on the go, maybe we could even directly append to the array as a buffer without intermediate copies and extra allocations as we do currently.

[jorgecarleitao]

I'm curious about the perf implications. Even for integers or dates, we will always need to verify that they are utf8 compliant to create valid StringArray value buffers. We could store then as BinaryArray instead. My hypothesis is that most of the to_str / to_datetime / to_int are not SIMDed and thus unlikely to benefit from Arrow, but it could also be that the columnar format helps the compiler.

Note that people can always pass a DataType::Utf8 to the schema instead of inferring it and cast the types themselves. I always understood the readers (csv, json, parquet) as ways to bypass that approach and create Arrow arrays directly from the format. It happens that CSV is a particularly poor format for this, as everything is a (not-necessarily utf8) string with very little invariants.

[Dandandan]

@jorgecarleitao note that the csvStringRecord also verifies whether strings are utf8. It adds a bit of overhead, but the utf8 checking itself is not much for now, it is mostly the logic surrounding StringRecord that adds the most overhead.
I think eventually we could use a StringArray or BinaryArray as buffer so we can remove the StringRecords which is internally a Vec<u8> (by using ByteRecord) and a Vec<usize> for the rows.

The current performance penalty between master and this branch currently is ~10% as we introduce an extra intermediate step which I think could be more than compensated for by removing the StringRecord abstraction, and trying to write to a string or binary array without intermediate steps.

This is the structure the csv crate is using per row:

structByteRecordInner{/// The position of this byte record.pos:Option<Position>,/// All fields in this record, stored contiguously.fields:Vec<u8>,/// The number of and location of each field in this record.bounds:Bounds,}

[jorgecarleitao]

Thanks for the explanation. I see what you mean. I agree that it is worth the effort to try it out 👍

[alamb]

I also like the general idea of using the cast kernel for consistency. Nice idea @Dandandan

[nevi-me]

• Implement cast that returns an error on parsing errors instead of null

See https://issues.apache.org/jira/browse/ARROW-7364

[alamb]

@Dandandan
What is the status of this PR?

As part of trying to clean up the backlog of Rust PRs in this repo, I am going through seemingly stale PRs and pinging the authors to see if there are any plans to continue the work or conversation.

[Dandandan]

Closing for now, will reopen when the other parts are done @alamb