Skip to content

Commit 19c60f0

Browse files
committed
fix review comments
1 parent ebddf05 commit 19c60f0

File tree

2 files changed

+8
-4
lines changed

2 files changed

+8
-4
lines changed

cpp/src/arrow/csv/writer.cc

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -634,6 +634,9 @@ class CSVWriterImpl : public ipc::RecordBatchWriter {
634634
memcpy(next, col_name.data(), col_name.size());
635635
next += col_name.size();
636636
break;
637+
// The behavior of the Needed quoting_style in CSV data depends on the data type.
638+
// And it is always quoted when the data type is binary. To avoid semantic
639+
// differences, the behavior of Need and AllValid should be consistent.
637640
case QuotingStyle::Needed:
638641
case QuotingStyle::AllValid:
639642
*next++ = '"';

cpp/src/arrow/csv/writer_test.cc

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -93,8 +93,9 @@ std::vector<WriterTestParams> GenerateTestCases() {
9393
auto dummy_schema = schema({field("a", uint8())});
9494
std::string dummy_batch_data = R"([{"a": null}])";
9595

96-
auto no_structral_header = schema({field("a ", uint64()), field("b", int32())});
97-
std::string expected_no_structral_header = std::string(R"(a ,b)") + "\n";
96+
auto header_without_structral_charaters =
97+
schema({field("a ", uint64()), field("b", int32())});
98+
std::string expected_header_without_structral_charaters = std::string(R"(a ,b)") + "\n";
9899
auto expected_status_no_quotes_with_structural_in_header = [](const char* header) {
99100
return Status::Invalid(
100101
"CSV header may not contain structural characters if quoting "
@@ -291,12 +292,12 @@ std::vector<WriterTestParams> GenerateTestCases() {
291292
DefaultTestOptions(/*include_header=*/false, /*null_string=*/"",
292293
QuotingStyle::Needed, /*eol=*/";", /*delimiter=*/';'),
293294
/*expected_output*/ "", expected_status_illegal_delimiter(';')},
294-
{no_structral_header, "[]",
295+
{header_without_structral_charaters, "[]",
295296
DefaultTestOptions(/*include_header=*/true, /*null_string=*/"",
296297
QuotingStyle::Needed, /*eol=*/"\n",
297298
/*delimiter=*/',', /*batch_size=*/5,
298299
/*quoting_header=*/QuotingStyle::None),
299-
expected_no_structral_header},
300+
expected_header_without_structral_charaters},
300301
{abc_schema, "[]",
301302
DefaultTestOptions(/*include_header=*/true, /*null_string=*/"",
302303
QuotingStyle::Needed, /*eol=*/"\n",

0 commit comments

Comments
 (0)