dmitry-chirkov-dremio commented on code in PR #54059:
URL: https://github.com/apache/arrow/pull/54059#discussion_r4349980313
##########
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:
With block_size=4, input "a,b\r1,2\r" has a complete header in the first
block. Withholding its CR sends only "a,b" to ProcessHeader, which fails.
After fixing the move issue below, I get this failure in serial, async and
streaming readers. The parent implementation reads the row correctly. Could we
preserve this case and add regression coverage?
--
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]
