lidavidm commented on a change in pull request #11863:
URL: https://github.com/apache/arrow/pull/11863#discussion_r763031350
##########
File path: cpp/src/arrow/csv/writer.cc
##########
@@ -126,13 +126,14 @@ class ColumnPopulator {
// Copies the contents of to out properly escaping any necessary characters.
// Returns the position prior to last copied character (out_end is
decremented).
-char* EscapeReverse(arrow::util::string_view s, char* out_end) {
+char* EscapeReverse(arrow::util::string_view s, char* out_end, bool escaping,
+ char escape_char) {
for (const char* val = s.data() + s.length() - 1; val >= s.data(); val--,
out_end--) {
- if (*val == '"') {
- *out_end = *val;
+ *out_end = *val;
+ if (escaping && *val == '"') {
Review comment:
Shouldn't we also test for the escape char?
##########
File path: cpp/src/arrow/csv/writer.cc
##########
@@ -126,13 +126,14 @@ class ColumnPopulator {
// Copies the contents of to out properly escaping any necessary characters.
// Returns the position prior to last copied character (out_end is
decremented).
-char* EscapeReverse(arrow::util::string_view s, char* out_end) {
+char* EscapeReverse(arrow::util::string_view s, char* out_end, bool escaping,
+ char escape_char) {
for (const char* val = s.data() + s.length() - 1; val >= s.data(); val--,
out_end--) {
- if (*val == '"') {
- *out_end = *val;
+ *out_end = *val;
+ if (escaping && *val == '"') {
Review comment:
(Though, once merged with #11849: that PR uses CountQuotes to check that
quotes don't appear when quoting is disabled, however, we need to make sure
that doesn't cause spurious errors if the escape character appears in an
unquoted cell. So looks like we need both CountQuotes and CountEscapes.)
##########
File path: cpp/src/arrow/csv/writer.cc
##########
@@ -126,13 +126,14 @@ class ColumnPopulator {
// Copies the contents of to out properly escaping any necessary characters.
// Returns the position prior to last copied character (out_end is
decremented).
-char* EscapeReverse(arrow::util::string_view s, char* out_end) {
+char* EscapeReverse(arrow::util::string_view s, char* out_end, bool escaping,
+ char escape_char) {
for (const char* val = s.data() + s.length() - 1; val >= s.data(); val--,
out_end--) {
- if (*val == '"') {
- *out_end = *val;
+ *out_end = *val;
+ if (escaping && *val == '"') {
Review comment:
Also CountEscapes (or CountQuotes) will also need to test for the escape
char.
##########
File path: cpp/src/arrow/csv/writer.cc
##########
@@ -184,9 +185,11 @@ class UnquotedColumnPopulator : public ColumnPopulator {
// a quote character (") and escaping is done my adding another quote.
class QuotedColumnPopulator : public ColumnPopulator {
Review comment:
Note the above docstring is outdated now.
--
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]
