crepererum commented on code in PR #700:
URL:
https://github.com/apache/arrow-rs-object-store/pull/700#discussion_r3145956340
##########
src/delimited.rs:
##########
@@ -77,22 +77,27 @@ impl LineDelimiter {
let is_escape = &mut self.is_escape;
let is_quote = &mut self.is_quote;
- let mut record_ends = val.iter().enumerate().filter_map(|(idx, v)| {
- if *is_escape {
- *is_escape = false;
- None
- } else if *v == ESCAPE {
- *is_escape = true;
- None
- } else if *v == QUOTE {
- *is_quote = !*is_quote;
- None
- } else if *is_quote {
- None
- } else {
- (*v == NEWLINE).then_some(idx + 1)
- }
- });
+ let mut record_ends = val
+ .iter()
+ .enumerate()
+ .filter_map(|(idx, v)| {
+ if *is_escape {
+ *is_escape = false;
+ None
+ } else if *v == ESCAPE {
+ *is_escape = true;
+ None
+ } else if *v == QUOTE {
+ *is_quote = !*is_quote;
+ None
+ } else if *is_quote {
+ None
+ } else {
+ (*v == NEWLINE).then_some(idx + 1)
+ }
+ })
+ .collect::<Vec<_>>()
+ .into_iter();
Review Comment:
```suggestion
// records_ends is a double ended iterator, so when calling
next_back() the quoting/escaping logic would run in reverse, corrupting the
internal state.
.collect::<Vec<_>>()
.into_iter();
```
(might need an additional line break, just copied your explanation from the
PR description)
It took me a while to understand why `collect().into_iter()` would be a
sensible choice here (because usually it isn't), so adding a code comment will
likely help a future reader.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
