AdityRajGupta commented on code in PR #54059:
URL: https://github.com/apache/arrow/pull/54059#discussion_r4388772252
##########
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:
I changed the reader now tracks the quoting state as it goes, following the
same rules as the parser (a quote only opens a field at a field start, ""
inside quotes is an escaped quote, escapes included). It only strips the
trailing CR off a joined buffer when the stream is cleanly at a line boundary
at that point. If the buffer ends inside a quoted field, it holds the whole
buffer instead and joins it with the next one, so the row stays in one piece
and the parser's own logic handles it.
<img width="802" height="498" alt="Screenshot 2026-10-27 at 6 10 26 PM"
src="https://github.com/user-attachments/assets/e2d3ca19-d8e8-4d80-919a-5a4ee5d7a970"
/>
Your exact case is covered in the tests now, in all three reader modes, and
the full suite is green on my side. Would appreciate another look when you have
a moment :)
--
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]