dmitry-chirkov-dremio commented on code in PR #54059:
URL: https://github.com/apache/arrow/pull/54059#discussion_r4369301994
##########
cpp/src/arrow/csv/reader.cc:
##########
@@ -120,25 +141,88 @@ class CSVBufferIterator {
first_buffer_ = false;
}
- if (trailing_cr_ && buf->data()[offset] == '\n') {
- // Skip '\r\n' line separator that started at the end of previous buffer
- ++offset;
+ // Fuse this block with whatever was withheld from the previous one, so
+ // that a CRLF pair straddling a block boundary stays contiguous inside
+ // a single buffer, where both the chunker and the parser handle it
+ // correctly -- whether it is a line separator or data inside a quoted
+ // field (GH-51368).
+ const int64_t incoming = buf->size() - offset;
+ bool fused = false;
+ std::shared_ptr<Buffer> current;
+ if (held_buffer_ != nullptr) {
+ auto held = std::move(held_buffer_);
+ ARROW_ASSIGN_OR_RAISE(auto joined, AllocateBuffer(held->size() + 1 +
incoming));
+ uint8_t* out = joined->mutable_data();
+ memcpy(out, held->data(), static_cast<size_t>(held->size()));
+ out[held->size()] = '\r';
+ if (incoming > 0) {
+ memcpy(out + held->size() + 1, buf->data() + offset,
+ static_cast<size_t>(incoming));
+ }
+ current = std::shared_ptr<Buffer>(std::move(joined));
+ fused = true;
+ } else if (pending_cr_) {
+ pending_cr_ = false;
+ ARROW_ASSIGN_OR_RAISE(auto joined, AllocateBuffer(1 + incoming));
+ uint8_t* out = joined->mutable_data();
+ out[0] = '\r';
+ if (incoming > 0) {
+ memcpy(out + 1, buf->data() + offset, static_cast<size_t>(incoming));
+ }
+ current = std::shared_ptr<Buffer>(std::move(joined));
+ fused = true;
+ } else if (offset != 0) {
+ current = SliceBuffer(buf, offset);
+ } else {
+ current = buf;
}
- trailing_cr_ = (buf->data()[buf->size() - 1] == '\r');
- buf = SliceBuffer(std::move(buf), offset);
- if (buf->size() == 0) {
- // EOF
+ if (current->size() > 0 && current->data()[current->size() - 1] == '\r') {
+ if (fused && incoming > 0) {
Review Comment:
The CR at the join can be inside a quoted field. With `block_size=8` and
`newlines_in_values=true`, this input still fails:
```
"a,b\r1,x\r2,\r3,\"a\rb\"\r"
```
I checked the parent and this commit locally. The parent reads three rows;
this version fails in serial, async and streaming readers. The last row is
exactly eight bytes.
Could we cover this case and avoid treating `fused` as proof that the buffer
contains a row terminator?
Also, perhaps ask your agent to take a step back and re-trace the steps and
re-evaluate the blind spots.
--
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]
