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



##########
File path: cpp/src/arrow/csv/writer.cc
##########
@@ -80,17 +80,19 @@ int64_t CountQuotes(util::string_view s) {
 // Matching quote pair character length.
 constexpr int64_t kQuoteCount = 2;
 constexpr int64_t kQuoteDelimiterCount = kQuoteCount + /*end_char*/ 1;
-static const std::string& str_comma = ",";
+constexpr char const* kStrComma = ",";
 
 // Interface for generating CSV data per column.
 // The intended usage is to iteratively call UpdateRowLengths for a column and
 // then PopulateColumns. PopulateColumns must be called in the reverse order 
of the
 // populators (it populates data backwards).
 class ColumnPopulator {
  public:
-  ColumnPopulator(MemoryPool* pool, const std::string& end_chars,
+  ColumnPopulator(MemoryPool* pool, const std::string end_chars,
                   std::shared_ptr<Buffer> null_string)
-      : end_chars_(end_chars), null_string_(std::move(null_string)), 
pool_(pool) {}
+      : end_chars_(std::move(end_chars)),

Review comment:
       std::string has may tricks to improve performance, e.g., short string 
optimization (SSO) to allocate short strings directly inside the object, not in 
a separated buffer. So short strings are not movable.
   You will see expected behaviour if you print string.size(). If it becomes 0, 
the string is moved, though the underlying buffer ~~may be not freed in case it 
may be used again soon~~ may be copied actually.




-- 
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]


Reply via email to