AdityRajGupta commented on code in PR #54059:
URL: https://github.com/apache/arrow/pull/54059#discussion_r4351153777
##########
cpp/src/arrow/csv/reader.cc:
##########
@@ -120,25 +130,53 @@ 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;
+ if (pending_cr_) {
+ // Reattach the '\r' withheld from the end of the previous buffer to the
+ // start of this one. This keeps a CRLF pair straddling the boundary
+ // 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).
+ pending_cr_ = false;
+ const int64_t rest = buf->size() - offset;
+ ARROW_ASSIGN_OR_RAISE(auto joined, AllocateBuffer(1 + rest));
+ uint8_t* out = joined->mutable_data();
+ out[0] = '\r';
+ if (rest > 0) {
+ memcpy(out + 1, buf->data() + offset, static_cast<size_t>(rest));
+ }
+ buf = std::shared_ptr<Buffer>(std::move(joined));
+ offset = 0;
+ }
+
+ if (buf->size() > offset && buf->data()[buf->size() - 1] == '\r') {
Review Comment:
Thanks for the catch. Yes the slice length could be evaluated after buf was
moved. I now calculate the slice length after moving buf and checked the
function for the same pattern. The quoted CRLF boundary regression test is
included, and the full CSV suite passed on Linux.
--
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]
