Copilot commented on code in PR #51286:
URL: https://github.com/apache/arrow/pull/51286#discussion_r3979812095
##########
cpp/src/arrow/csv/options.cc:
##########
@@ -23,8 +23,39 @@ namespace csv {
ParseOptions ParseOptions::Defaults() { return ParseOptions(); }
Status ParseOptions::Validate() const {
- if (ARROW_PREDICT_FALSE(delimiter == '\n' || delimiter == '\r')) {
- return Status::Invalid("ParseOptions: delimiter cannot be \\r or \\n");
+ // The chunker handles escapes before delimiter matching, so allowing the
escape
+ // character in a delimiter could make it disagree with the parser.
+ if (escaping) {
+ if (delimiter_string.empty()) {
+ if (delimiter == escape_char) {
+ return Status::Invalid(
+ "ParseOptions: delimiter cannot be the escape character when
escaping is "
+ "enabled");
+ }
+ } else if (delimiter_string.find(escape_char) != std::string::npos) {
+ return Status::Invalid(
+ "ParseOptions: delimiter_string cannot contain the escape character
when "
+ "escaping is enabled");
+ }
+ }
+ // Line endings delimit records and therefore cannot be part of a field
delimiter.
+ if (delimiter_string.empty()) {
+ if (ARROW_PREDICT_FALSE(delimiter == '\n' || delimiter == '\r')) {
+ return Status::Invalid("ParseOptions: delimiter cannot be \\r or \\n");
+ }
+ } else if (ARROW_PREDICT_FALSE(delimiter_string.find_first_of("\r\n") !=
+ std::string::npos)) {
+ return Status::Invalid("ParseOptions: delimiter_string cannot contain \\r
or \\n");
+ }
+ // The chunker handles quotes before delimiter matching at field boundaries,
so
+ // allowing the quote character at the start of a delimiter could make it
disagree
+ // with the parser.
+ const char delimiter_first_byte =
+ delimiter_string.empty() ? delimiter : delimiter_string.front();
+ if (ARROW_PREDICT_FALSE(quoting && delimiter_first_byte == quote_char)) {
+ return Status::Invalid(
+ "ParseOptions: delimiter cannot start with the quote character when
quoting is "
+ "enabled");
}
Review Comment:
The validation error message is now triggered for both `delimiter` and
`delimiter_string` (it checks the first byte of the effective delimiter), but
the message only mentions `delimiter`. This is confusing when a user sets
`delimiter_string` and hits this validation error.
--
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]
