pitrou commented on code in PR #51286:
URL: https://github.com/apache/arrow/pull/51286#discussion_r4440224700
##########
cpp/src/arrow/csv/parser_test.cc:
##########
@@ -268,67 +287,145 @@ TEST(BlockParser, Basics) {
}
}
-TEST(BlockParser, PadShortRows) {
+TEST(BlockParser, MultiDelimiter) {
auto options = ParseOptions::Defaults();
- options.pad_short_rows = true;
+ options.delimiter_string = "||";
- BlockParser parser(options, /*num_cols=*/3);
- AssertParseOk(parser, "1,2\n3,4,5\n");
- AssertColumnEq(parser, 0, {"1", "3"});
- AssertColumnEq(parser, 1, {"2", "4"});
- std::vector<std::string> values;
- std::vector<bool> missing;
- ASSERT_OK(parser.VisitColumn(
- 2, [&](const uint8_t* data, uint32_t size, bool, bool is_missing) ->
Status {
- values.emplace_back(reinterpret_cast<const char*>(data), size);
- missing.push_back(is_missing);
- return Status::OK();
- }));
- ASSERT_EQ(values, std::vector<std::string>({"", "5"}));
- ASSERT_EQ(missing, std::vector<bool>({true, false}));
-
- BlockParser last_row_parser(options, /*num_cols=*/3);
- AssertParseOk(last_row_parser, "1,2\n");
- std::vector<bool> last_row_missing;
- ASSERT_OK(last_row_parser.VisitLastRow(
- [&](const uint8_t*, uint32_t, bool, bool is_missing) -> Status {
- last_row_missing.push_back(is_missing);
- return Status::OK();
- }));
- ASSERT_EQ(last_row_missing, std::vector<bool>({false, false, true}));
+ {
+ // Parse multiple input views with unmatched pipes, a quoted delimiter, and
+ // a final row without a newline.
+ BlockParser parser(options);
+ AssertParseFinal(parser, Views({"|a||b|c||d|\n", "e||\"f||g\"||h"}));
+ AssertColumnsEq(parser, {{"|a", "e"}, {"b|c", "f||g"}, {"d|", "h"}},
+ {{false, false}, {false, true}, {false, false}});
+ }
+ {
+ // Recognize a quoted field with an embedded newline after a delimiter
mismatch.
+ BlockParser parser(options);
+ AssertParseFinal(parser, "a|b||\"c\n d\"||e\n");
+ AssertColumnsEq(parser, {{"a|b"}, {"c\n d"}, {"e"}}, {{false}, {true},
{false}});
+ }
+ {
+ // Leading, consecutive, and trailing delimiters produce empty fields at
EOF.
+ BlockParser parser(options);
+ AssertParseFinal(parser, "||a|b||||c||");
+ AssertColumnsEq(parser, {{""}, {"a|b"}, {""}, {"c"}, {""}});
+ }
+
+ options.escaping = true;
+ {
+ BlockParser parser(options);
+ AssertParseFinal(parser, R"(a\|||b|c||"d\||e")");
+ AssertColumnsEq(parser, {{"a|"}, {"b|c"}, {"d||e"}}, {{false}, {false},
{true}});
+ }
}
-TEST(BlockParser, IgnoreExtraColumns) {
+TEST(BlockParser, DelimiterPrefix) {
auto options = ParseOptions::Defaults();
- options.ignore_extra_columns = true;
+ options.delimiter_string = "||";
BlockParser parser(options, /*num_cols=*/2);
- AssertParseOk(parser, "a,\"b\",c,\nd,e\n");
- AssertColumnsEq(parser, {{"a", "d"}, {"b", "e"}}, {{false, false}, {true,
false}});
-
- BlockParser final_parser(options, /*num_cols=*/2);
- AssertParseFinal(final_parser, "a,b,");
- AssertColumnsEq(final_parser, {{"a"}, {"b"}});
+ AssertParsePartial(parser, "a||b|", 0);
+ AssertParseFinal(parser, "a||b|");
+ AssertColumnsEq(parser, {{"a"}, {"b|"}});
}
-TEST(BlockParser, PadAndIgnore) {
+TEST(BlockParser, SingleCharacterDelimiterString) {
auto options = ParseOptions::Defaults();
- options.pad_short_rows = true;
- options.ignore_extra_columns = true;
+ options.delimiter = ',';
+ options.delimiter_string = "|";
- BlockParser parser(options, /*num_cols=*/2);
- AssertParseFinal(parser, "a,b,c\nd");
- AssertColumnEq(parser, 0, {"a", "d"});
- std::vector<std::string> values;
- std::vector<bool> missing;
- ASSERT_OK(parser.VisitColumn(
- 1, [&](const uint8_t* data, uint32_t size, bool, bool is_missing) ->
Status {
- values.emplace_back(reinterpret_cast<const char*>(data), size);
- missing.push_back(is_missing);
- return Status::OK();
- }));
- ASSERT_EQ(values, std::vector<std::string>({"b", ""}));
- ASSERT_EQ(missing, std::vector<bool>({false, true}));
+ BlockParser parser(options);
+ AssertParseFinal(parser, "a|b\n");
+ AssertColumnsEq(parser, {{"a"}, {"b"}});
+}
+
+TEST(BlockParser, PadShortRows) {
+ for (const std::string delimiter : {",", "||"}) {
+ ARROW_SCOPED_TRACE("delimiter = ", delimiter);
+ auto options = ParseOptions::Defaults();
+ options.delimiter_string = delimiter == "," ? "" : delimiter;
+ const std::string short_row = "1" + delimiter + "2\n";
+
+ options.pad_short_rows = true;
+ BlockParser parser(options, /*num_cols=*/3);
+ AssertParseOk(parser, short_row + "3" + delimiter + "4" + delimiter +
"5\n6" +
+ delimiter + "7" + delimiter + "\n");
+ AssertColumnEq(parser, 0, {"1", "3", "6"});
+ AssertColumnEq(parser, 1, {"2", "4", "7"});
Review Comment:
`AssertColumnEqWithMissing` as well here?
##########
cpp/src/arrow/csv/chunker_test.cc:
##########
@@ -193,6 +193,49 @@ TEST_P(BaseChunkerTest, QuotingNewline) {
}
}
+TEST_P(BaseChunkerTest, MultiDelimiter) {
+ if (!options_.newlines_in_values) {
+ GTEST_SKIP() << "Quoted newlines require newlines_in_values";
+ }
+ options_.delimiter_string = "||";
+ options_.escaping = true;
+ MakeChunker();
+
+ // The escape breaks the delimiter prefix, so the quote before "me" is
literal.
+ auto partial = std::make_shared<Buffer>("na|\\x|\"me|");
+ auto block =
std::make_shared<Buffer>("|mes|sage||\"hello\nworld\"||42\nnext||row\n");
+ std::shared_ptr<Buffer> completion;
+ std::shared_ptr<Buffer> rest;
+ ASSERT_OK(chunker_->ProcessWithPartial(partial, block, &completion, &rest));
+ ASSERT_EQ(completion->ToString(), "|mes|sage||\"hello\nworld\"||42\n");
+ ASSERT_EQ(rest->ToString(), "next||row\n");
+
+ partial = std::make_shared<Buffer>("name|");
+ block = std::make_shared<Buffer>("|message");
+ ASSERT_RAISES(Invalid,
+ chunker_->ProcessWithPartial(partial, block, &completion,
&rest));
+
+ // Long ordinary fields make the bulk filter profitable.
+ for (size_t padding : {0, 256}) {
+ const std::string first_field = std::string(padding, 'a') + "a|b";
+ const std::string row = first_field + "||\"c\n d\"||e\n";
+ const std::string next_row = std::string(padding, 'n') + "next||row\n";
+ MakeChunker();
+ const auto lengths = {row.size(), next_row.size()};
+ AssertChunking(*chunker_, row + next_row, lengths);
+
+ for (size_t split :
+ {first_field.find('|') + 1, first_field.size() + 1,
first_field.size() + 2}) {
Review Comment:
Why not test all values from 1 to `row.size() - 1`?
##########
cpp/src/arrow/csv/parser_test.cc:
##########
@@ -268,67 +287,145 @@ TEST(BlockParser, Basics) {
}
}
-TEST(BlockParser, PadShortRows) {
+TEST(BlockParser, MultiDelimiter) {
auto options = ParseOptions::Defaults();
- options.pad_short_rows = true;
+ options.delimiter_string = "||";
- BlockParser parser(options, /*num_cols=*/3);
- AssertParseOk(parser, "1,2\n3,4,5\n");
- AssertColumnEq(parser, 0, {"1", "3"});
- AssertColumnEq(parser, 1, {"2", "4"});
- std::vector<std::string> values;
- std::vector<bool> missing;
- ASSERT_OK(parser.VisitColumn(
- 2, [&](const uint8_t* data, uint32_t size, bool, bool is_missing) ->
Status {
- values.emplace_back(reinterpret_cast<const char*>(data), size);
- missing.push_back(is_missing);
- return Status::OK();
- }));
- ASSERT_EQ(values, std::vector<std::string>({"", "5"}));
- ASSERT_EQ(missing, std::vector<bool>({true, false}));
-
- BlockParser last_row_parser(options, /*num_cols=*/3);
- AssertParseOk(last_row_parser, "1,2\n");
- std::vector<bool> last_row_missing;
- ASSERT_OK(last_row_parser.VisitLastRow(
- [&](const uint8_t*, uint32_t, bool, bool is_missing) -> Status {
- last_row_missing.push_back(is_missing);
- return Status::OK();
- }));
- ASSERT_EQ(last_row_missing, std::vector<bool>({false, false, true}));
+ {
+ // Parse multiple input views with unmatched pipes, a quoted delimiter, and
+ // a final row without a newline.
+ BlockParser parser(options);
+ AssertParseFinal(parser, Views({"|a||b|c||d|\n", "e||\"f||g\"||h"}));
+ AssertColumnsEq(parser, {{"|a", "e"}, {"b|c", "f||g"}, {"d|", "h"}},
+ {{false, false}, {false, true}, {false, false}});
+ }
+ {
+ // Recognize a quoted field with an embedded newline after a delimiter
mismatch.
+ BlockParser parser(options);
+ AssertParseFinal(parser, "a|b||\"c\n d\"||e\n");
+ AssertColumnsEq(parser, {{"a|b"}, {"c\n d"}, {"e"}}, {{false}, {true},
{false}});
+ }
+ {
+ // Leading, consecutive, and trailing delimiters produce empty fields at
EOF.
+ BlockParser parser(options);
+ AssertParseFinal(parser, "||a|b||||c||");
+ AssertColumnsEq(parser, {{""}, {"a|b"}, {""}, {"c"}, {""}});
+ }
+
+ options.escaping = true;
+ {
+ BlockParser parser(options);
+ AssertParseFinal(parser, R"(a\|||b|c||"d\||e")");
+ AssertColumnsEq(parser, {{"a|"}, {"b|c"}, {"d||e"}}, {{false}, {false},
{true}});
+ }
}
-TEST(BlockParser, IgnoreExtraColumns) {
+TEST(BlockParser, DelimiterPrefix) {
auto options = ParseOptions::Defaults();
- options.ignore_extra_columns = true;
+ options.delimiter_string = "||";
BlockParser parser(options, /*num_cols=*/2);
- AssertParseOk(parser, "a,\"b\",c,\nd,e\n");
- AssertColumnsEq(parser, {{"a", "d"}, {"b", "e"}}, {{false, false}, {true,
false}});
-
- BlockParser final_parser(options, /*num_cols=*/2);
- AssertParseFinal(final_parser, "a,b,");
- AssertColumnsEq(final_parser, {{"a"}, {"b"}});
+ AssertParsePartial(parser, "a||b|", 0);
+ AssertParseFinal(parser, "a||b|");
+ AssertColumnsEq(parser, {{"a"}, {"b|"}});
}
-TEST(BlockParser, PadAndIgnore) {
+TEST(BlockParser, SingleCharacterDelimiterString) {
auto options = ParseOptions::Defaults();
- options.pad_short_rows = true;
- options.ignore_extra_columns = true;
+ options.delimiter = ',';
+ options.delimiter_string = "|";
- BlockParser parser(options, /*num_cols=*/2);
- AssertParseFinal(parser, "a,b,c\nd");
- AssertColumnEq(parser, 0, {"a", "d"});
- std::vector<std::string> values;
- std::vector<bool> missing;
- ASSERT_OK(parser.VisitColumn(
- 1, [&](const uint8_t* data, uint32_t size, bool, bool is_missing) ->
Status {
- values.emplace_back(reinterpret_cast<const char*>(data), size);
- missing.push_back(is_missing);
- return Status::OK();
- }));
- ASSERT_EQ(values, std::vector<std::string>({"b", ""}));
- ASSERT_EQ(missing, std::vector<bool>({false, true}));
+ BlockParser parser(options);
+ AssertParseFinal(parser, "a|b\n");
+ AssertColumnsEq(parser, {{"a"}, {"b"}});
+}
+
+TEST(BlockParser, PadShortRows) {
+ for (const std::string delimiter : {",", "||"}) {
+ ARROW_SCOPED_TRACE("delimiter = ", delimiter);
+ auto options = ParseOptions::Defaults();
+ options.delimiter_string = delimiter == "," ? "" : delimiter;
+ const std::string short_row = "1" + delimiter + "2\n";
+
+ options.pad_short_rows = true;
+ BlockParser parser(options, /*num_cols=*/3);
+ AssertParseOk(parser, short_row + "3" + delimiter + "4" + delimiter +
"5\n6" +
+ delimiter + "7" + delimiter + "\n");
+ AssertColumnEq(parser, 0, {"1", "3", "6"});
+ AssertColumnEq(parser, 1, {"2", "4", "7"});
+ AssertColumnEqWithMissing(parser, 2, {"", "5", ""}, {true, false, false});
+
+ BlockParser final_parser(options, /*num_cols=*/3);
+ AssertParseFinal(final_parser, "6" + delimiter + "7" + delimiter);
+ AssertColumnsEq(final_parser, {{"6"}, {"7"}, {""}});
Review Comment:
`AssertColumnEqWithMissing` too?
##########
cpp/src/arrow/csv/parser_test.cc:
##########
@@ -268,67 +287,145 @@ TEST(BlockParser, Basics) {
}
}
-TEST(BlockParser, PadShortRows) {
+TEST(BlockParser, MultiDelimiter) {
auto options = ParseOptions::Defaults();
- options.pad_short_rows = true;
+ options.delimiter_string = "||";
- BlockParser parser(options, /*num_cols=*/3);
- AssertParseOk(parser, "1,2\n3,4,5\n");
- AssertColumnEq(parser, 0, {"1", "3"});
- AssertColumnEq(parser, 1, {"2", "4"});
- std::vector<std::string> values;
- std::vector<bool> missing;
- ASSERT_OK(parser.VisitColumn(
- 2, [&](const uint8_t* data, uint32_t size, bool, bool is_missing) ->
Status {
- values.emplace_back(reinterpret_cast<const char*>(data), size);
- missing.push_back(is_missing);
- return Status::OK();
- }));
- ASSERT_EQ(values, std::vector<std::string>({"", "5"}));
- ASSERT_EQ(missing, std::vector<bool>({true, false}));
-
- BlockParser last_row_parser(options, /*num_cols=*/3);
- AssertParseOk(last_row_parser, "1,2\n");
- std::vector<bool> last_row_missing;
- ASSERT_OK(last_row_parser.VisitLastRow(
- [&](const uint8_t*, uint32_t, bool, bool is_missing) -> Status {
- last_row_missing.push_back(is_missing);
- return Status::OK();
- }));
- ASSERT_EQ(last_row_missing, std::vector<bool>({false, false, true}));
+ {
+ // Parse multiple input views with unmatched pipes, a quoted delimiter, and
+ // a final row without a newline.
+ BlockParser parser(options);
+ AssertParseFinal(parser, Views({"|a||b|c||d|\n", "e||\"f||g\"||h"}));
+ AssertColumnsEq(parser, {{"|a", "e"}, {"b|c", "f||g"}, {"d|", "h"}},
+ {{false, false}, {false, true}, {false, false}});
+ }
+ {
+ // Recognize a quoted field with an embedded newline after a delimiter
mismatch.
+ BlockParser parser(options);
+ AssertParseFinal(parser, "a|b||\"c\n d\"||e\n");
+ AssertColumnsEq(parser, {{"a|b"}, {"c\n d"}, {"e"}}, {{false}, {true},
{false}});
+ }
+ {
+ // Leading, consecutive, and trailing delimiters produce empty fields at
EOF.
+ BlockParser parser(options);
+ AssertParseFinal(parser, "||a|b||||c||");
+ AssertColumnsEq(parser, {{""}, {"a|b"}, {""}, {"c"}, {""}});
+ }
+
+ options.escaping = true;
+ {
+ BlockParser parser(options);
+ AssertParseFinal(parser, R"(a\|||b|c||"d\||e")");
+ AssertColumnsEq(parser, {{"a|"}, {"b|c"}, {"d||e"}}, {{false}, {false},
{true}});
+ }
}
-TEST(BlockParser, IgnoreExtraColumns) {
+TEST(BlockParser, DelimiterPrefix) {
auto options = ParseOptions::Defaults();
- options.ignore_extra_columns = true;
+ options.delimiter_string = "||";
BlockParser parser(options, /*num_cols=*/2);
- AssertParseOk(parser, "a,\"b\",c,\nd,e\n");
- AssertColumnsEq(parser, {{"a", "d"}, {"b", "e"}}, {{false, false}, {true,
false}});
-
- BlockParser final_parser(options, /*num_cols=*/2);
- AssertParseFinal(final_parser, "a,b,");
- AssertColumnsEq(final_parser, {{"a"}, {"b"}});
+ AssertParsePartial(parser, "a||b|", 0);
+ AssertParseFinal(parser, "a||b|");
+ AssertColumnsEq(parser, {{"a"}, {"b|"}});
}
-TEST(BlockParser, PadAndIgnore) {
+TEST(BlockParser, SingleCharacterDelimiterString) {
auto options = ParseOptions::Defaults();
- options.pad_short_rows = true;
- options.ignore_extra_columns = true;
+ options.delimiter = ',';
+ options.delimiter_string = "|";
- BlockParser parser(options, /*num_cols=*/2);
- AssertParseFinal(parser, "a,b,c\nd");
- AssertColumnEq(parser, 0, {"a", "d"});
- std::vector<std::string> values;
- std::vector<bool> missing;
- ASSERT_OK(parser.VisitColumn(
- 1, [&](const uint8_t* data, uint32_t size, bool, bool is_missing) ->
Status {
- values.emplace_back(reinterpret_cast<const char*>(data), size);
- missing.push_back(is_missing);
- return Status::OK();
- }));
- ASSERT_EQ(values, std::vector<std::string>({"b", ""}));
- ASSERT_EQ(missing, std::vector<bool>({false, true}));
+ BlockParser parser(options);
+ AssertParseFinal(parser, "a|b\n");
+ AssertColumnsEq(parser, {{"a"}, {"b"}});
+}
+
+TEST(BlockParser, PadShortRows) {
+ for (const std::string delimiter : {",", "||"}) {
+ ARROW_SCOPED_TRACE("delimiter = ", delimiter);
+ auto options = ParseOptions::Defaults();
+ options.delimiter_string = delimiter == "," ? "" : delimiter;
+ const std::string short_row = "1" + delimiter + "2\n";
+
+ options.pad_short_rows = true;
+ BlockParser parser(options, /*num_cols=*/3);
+ AssertParseOk(parser, short_row + "3" + delimiter + "4" + delimiter +
"5\n6" +
+ delimiter + "7" + delimiter + "\n");
+ AssertColumnEq(parser, 0, {"1", "3", "6"});
+ AssertColumnEq(parser, 1, {"2", "4", "7"});
+ AssertColumnEqWithMissing(parser, 2, {"", "5", ""}, {true, false, false});
+
+ BlockParser final_parser(options, /*num_cols=*/3);
+ AssertParseFinal(final_parser, "6" + delimiter + "7" + delimiter);
+ AssertColumnsEq(final_parser, {{"6"}, {"7"}, {""}});
+
+ // Padding short rows must not accept extra columns.
+ BlockParser wide_parser(options, /*num_cols=*/2);
+ uint32_t parsed_size;
+ EXPECT_RAISES_WITH_MESSAGE_THAT(
+ Invalid, testing::HasSubstr("Expected 2 columns, got 4"),
+ ParseFinal(wide_parser,
+ "a" + delimiter + "b" + delimiter + "\"extra" + delimiter +
"field\"" +
+ delimiter,
+ &parsed_size));
+
+ BlockParser last_row_parser(options, /*num_cols=*/3);
+ AssertParseOk(last_row_parser, short_row);
+ std::vector<bool> last_row_missing;
+ ASSERT_OK(last_row_parser.VisitLastRow(
+ [&](const uint8_t*, uint32_t, bool, bool is_missing) -> Status {
+ last_row_missing.push_back(is_missing);
+ return Status::OK();
+ }));
+ ASSERT_EQ(last_row_missing, std::vector<bool>({false, false, true}));
+ }
+}
+
+TEST(BlockParser, IgnoreExtraColumns) {
+ for (const std::string delimiter : {",", "||"}) {
+ ARROW_SCOPED_TRACE("delimiter = ", delimiter);
+ auto options = ParseOptions::Defaults();
+ options.delimiter_string = delimiter == "," ? "" : delimiter;
+ options.ignore_extra_columns = true;
+ const std::string wide_row =
+ "a" + delimiter + "b" + delimiter + "\"extra" + delimiter + "field\"";
+
+ BlockParser parser(options, /*num_cols=*/2);
+ AssertParseOk(parser, "a" + delimiter + "\"b\"" + delimiter + "c" +
delimiter +
+ "\nd" + delimiter + "e\n");
+ AssertColumnsEq(parser, {{"a", "d"}, {"b", "e"}}, {{false, false}, {true,
false}});
+
+ BlockParser final_parser(options, /*num_cols=*/2);
+ AssertParseFinal(final_parser, wide_row + delimiter);
+ AssertColumnsEq(final_parser, {{"a"}, {"b"}});
+
+ // Ignoring extra columns must not accept short rows.
+ uint32_t parsed_size;
+ BlockParser short_parser(options, /*num_cols=*/2);
+ EXPECT_RAISES_WITH_MESSAGE_THAT(
+ Invalid, testing::HasSubstr("Expected 2 columns, got 1: c"),
+ ParseFinal(short_parser, wide_row + "\nc\n", &parsed_size));
+ }
+}
+
+TEST(BlockParser, PadAndIgnore) {
+ for (const std::string delimiter : {",", "||"}) {
+ ARROW_SCOPED_TRACE("delimiter = ", delimiter);
+ auto options = ParseOptions::Defaults();
+ options.delimiter_string = delimiter == "," ? "" : delimiter;
+ options.pad_short_rows = true;
+ options.ignore_extra_columns = true;
+
+ BlockParser parser(options, /*num_cols=*/2);
+ AssertParseFinal(parser, "a" + delimiter + "b" + delimiter + "\"extra" +
delimiter +
+ "field\"\nd\ne" + delimiter);
+ AssertColumnEq(parser, 0, {"a", "d", "e"});
Review Comment:
`AssertColumnEqWithMissing`?
--
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]
