Building a StringRecord from a raw line #326
-
|
It seems that currently there's no API to serde-deserialize raw records that are retrieved individually. My use-case is basically something like: #[derive(Deserialize)]
struct S<'a> {
a: &'a str,
b: &'a str,
}
let header: StringRecord = ["a", "b"].into_iter().collect(); // Constant header
let mut raw_record = csv::StringRecord::new(); // Reuse across iterations to avoid allocating
while let Some(record_str) = stream.await { // Note how this doesn't fit with Reader
raw_record.read_raw(record_str); // This doesn't exist (question is how to set settings of course...)
let deserialized: S = raw_record.deserialize(Some(&headers))?;
// And do stuff with this
} |
Beta Was this translation helpful? Give feedback.
Replies: 2 comments 9 replies
-
|
Possible workaround: #[tokio::main]
async fn main() {
tokio::task::spawn(async {
let mut csv_reader: csv::Reader<Cursor<Box<[u8]>>> = csv::ReaderBuilder::new()
.has_headers(false)
.from_reader(Cursor::new(Box::from([])));
let mut string_record = csv::StringRecord::new();
let headers = csv::StringRecord::from(vec!["a", "b", "c"]);
let mut lines = stream::iter(["a,b,c", "d,e,f", "g,h,i"]);
while let Some(l) = lines.next().await {
*csv_reader.get_mut() = Cursor::new(Box::from(l.as_bytes()));
#[derive(serde_derive::Deserialize, Debug)]
struct S<'a> {
a: &'a str,
b: &'a str,
c: &'a str,
}
assert!(csv_reader.read_record(&mut string_record)?);
let deserialized: S = string_record.deserialize(Some(&headers))?;
dbg!(deserialized);
}
Ok::<_, anyhow::Error>(())
}).unwrap();
}This seems pretty dirty: it seems pretty uncertain that this won't break on future Also, it uses the internal reader's BufReader as intermediate buffer, which creates unnecessary additional copies -> overhead.
Test to add to the test suite if this workaround is actually guaranteed behavior: #[test]
fn test_csv_line_by_line_feeding() {
let mut csv_reader: csv::Reader<Cursor<Box<[u8]>>> = csv::ReaderBuilder::new()
.has_headers(false)
.from_reader(Cursor::new(Box::from([])));
let mut string_record = csv::StringRecord::new();
let headers = csv::StringRecord::from(vec!["a", "b", "c"]);
#[derive(serde_derive::Deserialize, Debug, PartialEq, Eq)]
struct S<'a> {
a: &'a str,
b: &'a str,
c: &'a str,
}
let input = ["a,b,c", "d,e,f\n\naaaaa\n", "g,h,i\n"];
let expected_out = [
S { a: "a", b: "b", c: "c" },
S { a: "d", b: "e", c: "f" },
S { a: "g", b: "h", c: "i" },
];
assert_eq!(input.len(), expected_out.len());
for (line, expected_out) in input.iter().zip(expected_out) {
*csv_reader.get_mut() = Cursor::new(Box::from(line.as_bytes()));
csv_reader.seek(csv::Position::new()).unwrap(); // This will reset the reader state
assert!(csv_reader.read_record(&mut string_record).unwrap());
let deserialized: S = string_record.deserialize(Some(&headers)).unwrap();
assert_eq!(deserialized, expected_out);
}
} |
Beta Was this translation helpful? Give feedback.
-
|
I can't make heads or tails of what you're trying to do. But If you want to parse a single CSV record from a single line, then you need to create a But more generally, "parse a single CSV record from a single line" is incorrect because a CSV record can span multiple lines. |
Beta Was this translation helpful? Give feedback.
-
|
If there is a simple change that will let you do what you want with csv-core then I'm open to that. Please open a PR. I won't necessarily rubber stamp exposing private APIs if it inhibits API evolution, but we can look at a proposal in detail. |
Beta Was this translation helpful? Give feedback.
-
The other main difference is that it reads from a provided slice, which would allow feeding those one by one.
Yes I have, it doesn't prevent me from reaching the million lines per second, and is also a good compromise with regards to behavior evolution/code readability/performance (headers can be dynamically parametrized, but struct into which we're parsing can't). While I don't think the extra copy I pay here is going to prevent that either, it seems that it comes with no code readability benefits however, so that doesn't feel like an optimal solution (but it may still be the best compromise for the reasons you mentioned).
Either of these two changes would allow avoiding this extra copy AFAICT:
I have no particular preference on which would be best. However given the two rather large downsides with regards to API evolution (as you said, it seems this use-case doesn't fit very well with the Should I open a PR for this? |
Beta Was this translation helpful? Give feedback.
-
I still think you should do this: fn main() {
let raw = r"foo,bar,baz";
let mut sr = csv::StringRecord::new();
csv::ReaderBuilder::new()
.has_headers(false)
.from_reader(raw.as_bytes())
.read_record(&mut sr)
.unwrap();
dbg!(&sr);
}You've presumably concluded that you don't want the overhead of building a reader, but it's not clear to me it's actually significant, especially if you're using Serde. You say it is though. I guess I have to take your word for it. Maybe there is a way to make building a
Okay, so looking at this, I don't think that's doable. It exposes the internal implementation details of a
This is also definitely not going to happen. There's no way I'm ever going to make
Changing the underlying reader out from underneath the The other alternative here is to just use The whole point of |
Beta Was this translation helpful? Give feedback.
-
|
One other alternative in the design space here is to split apart "CSV reader configuration" and "CSV reader building." Then you could expose a |
Beta Was this translation helpful? Give feedback.
-
Yeah I've thought about that option as well, and figured that it seems that would mean refactoring a big part of the reader out of itself into Config. I'm not sure that would be worth doing either for such a niche use-case. (If you happen to have time to review such big changes it would probably be better spent unblocking the serde flatten support PR 😅)
It seems that given the current signatures of csv_core it could hardly behave differently. The advantage of going through the Reader is that checks such as "flexible" are maintained, and code is simpler, compared to the version where one would manually populate the StringRecord using csv_core. |
Beta Was this translation helpful? Give feedback.
I can't make heads or tails of what you're trying to do. But
Reader::read_recordis the way to read a single record.If you want to parse a single CSV record from a single line, then you need to create a
Readerfor that single line.But more generally, "parse a single CSV record from a single line" is incorrect because a CSV record can span multiple lines.