westonpace commented on a change in pull request #11857:
URL: https://github.com/apache/arrow/pull/11857#discussion_r766264173
##########
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:
I had to look it up. You are seeing the effects of "short string mode":
https://joellaity.com/2020/01/31/string.html
If strings are short enough they are basically stack allocated. There is no
real difference between a move and a copy of a short string.
If you make your strings longer than 22 characters then you will see
expected results.
This means, in this case, the moves are not really necessary, but still it
is nice to keep them for consistency's sake I think.
--
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]
