[GitHub] [arrow] cyb70289 commented on a change in pull request #12574: ARROW-15854: [C++] Refine CSV writer code

[GitBox]

cyb70289 commented on a change in pull request #12574:
URL: https://github.com/apache/arrow/pull/12574#discussion_r821286290



##########
File path: cpp/src/arrow/csv/writer.cc
##########
@@ -184,28 +181,23 @@ class UnquotedColumnPopulator : public ColumnPopulator {
     return Status::OK();
   }
 
-  Status PopulateColumns(char* output, int32_t* offsets) const override {
+  Status PopulateRows(char* output, int64_t* offsets) const override {
     // Function applied to valid values cast to string.
     auto valid_function = [&](arrow::util::string_view s) {
-      int32_t next_column_offset = static_cast<int32_t>(s.length() + 
end_chars_.size());
-      memcpy((output + *offsets - next_column_offset), s.data(), s.length());
-      memcpy((output + *offsets - end_chars_.size()), end_chars_.c_str(),
-             end_chars_.size());
-      *offsets -= next_column_offset;
+      memcpy(output + *offsets, s.data(), s.length());
+      memcpy(output + *offsets + s.length(), end_chars_.c_str(), 
end_chars_.size());
+      *offsets += static_cast<int64_t>(s.length() + end_chars_.size());
       offsets++;
       return Status::OK();
     };
 
     // Function applied to null values cast to string.
     auto null_function = [&]() {
       // For nulls, the configured null value string is copied into the output.
-      int32_t next_column_offset =
-          static_cast<int32_t>(null_string_->size() + end_chars_.size());
-      memcpy((output + *offsets - next_column_offset), null_string_->data(),
-             null_string_->size());
-      memcpy((output + *offsets - end_chars_.size()), end_chars_.c_str(),
+      memcpy(output + *offsets, null_string_->data(), null_string_->size());
+      memcpy(output + *offsets + null_string_->size(), end_chars_.c_str(),
              end_chars_.size());
-      *offsets -= next_column_offset;
+      *offsets += static_cast<int32_t>(null_string_->size() + 
end_chars_.size());

Review comment:
       changed

##########
File path: cpp/src/arrow/csv/writer.cc
##########
@@ -309,40 +300,33 @@ class QuotedColumnPopulator : public ColumnPopulator {
     return Status::OK();
   }
 
-  Status PopulateColumns(char* output, int32_t* offsets) const override {
+  Status PopulateRows(char* output, int64_t* offsets) const override {
     auto needs_escaping = row_needs_escaping_.begin();
     VisitArrayDataInline<StringType>(
         *(casted_array_->data()),
         [&](arrow::util::string_view s) {
           // still needs string content length to be added
-          char* row_end = output + *offsets;
-          int32_t next_column_offset = 0;
+          char* row = output + *offsets;
+          *row++ = '"';
           if (!*needs_escaping) {
-            next_column_offset = static_cast<int32_t>(s.length() + 
kQuoteDelimiterCount);
-            memcpy(row_end - next_column_offset + /*quote_offset=*/1, s.data(),
-                   s.length());
+            memcpy(row, s.data(), s.length());
+            row += s.length();
           } else {
-            // Adjust row_end by 2 + end_chars_.size(): 1 quote char, 
end_chars_.size()
-            // and 1 to position at the first position to write to.
-            next_column_offset = static_cast<int32_t>(
-                row_end - EscapeReverse(s, row_end - 2 - end_chars_.size()));
+            row = Escape(s, row);
           }
-          *(row_end - next_column_offset) = '"';
-          *(row_end - end_chars_.size() - 1) = '"';
-          memcpy(row_end - end_chars_.size(), end_chars_.data(), 
end_chars_.length());
-          *offsets -= next_column_offset;
+          *row++ = '"';
+          memcpy(row, end_chars_.data(), end_chars_.length());
+          row += end_chars_.length();
+          *offsets += static_cast<int64_t>(row - (output + *offsets));
           offsets++;
           needs_escaping++;
         },
         [&]() {
           // For nulls, the configured null value string is copied into the 
output.
-          int32_t next_column_offset =
-              static_cast<int32_t>(null_string_->size() + end_chars_.size());
-          memcpy((output + *offsets - next_column_offset), 
null_string_->data(),
-                 null_string_->size());
-          memcpy((output + *offsets - end_chars_.size()), end_chars_.c_str(),
+          memcpy(output + *offsets, null_string_->data(), 
null_string_->size());
+          memcpy(output + *offsets + null_string_->size(), end_chars_.c_str(),
                  end_chars_.size());
-          *offsets -= next_column_offset;
+          *offsets += static_cast<int32_t>(null_string_->size() + 
end_chars_.size());

Review comment:
       changed




-- 
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: github-unsubscr...@arrow.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org