Uh oh!
There was an error while loading. Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork 4.3k
GH-50925: [C++] Allow CSV reader to pad rows with missing trailing fields#50926
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Uh oh!
There was an error while loading. Please reload this page.
Changes from all commits
File filter
Filter by extension
Conversations
Uh oh!
There was an error while loading. Please reload this page.
Jump to
Uh oh!
There was an error while loading. Please reload this page.
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -61,6 +61,8 @@ struct ARROW_EXPORT ParseOptions { | ||
| bool ignore_empty_lines = true; | ||
| /// A handler function for rows which do not have the correct number of columns | ||
| InvalidRowHandler invalid_row_handler; | ||
| /// Whether rows with fewer columns than expected are padded with nulls. | ||
| bool pad_short_rows = false; | ||
wgtmac marked this conversation as resolved.
Uh oh!There was an error while loading. Please reload this page. | ||
| /// Create parsing options with default values | ||
| static ParseOptions Defaults(); | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -22,6 +22,7 @@ | ||
| #include <cstdint> | ||
| #include <memory> | ||
| #include <string_view> | ||
| #include <utility> | ||
| #include <vector> | ||
| #include "arrow/buffer.h" | ||
| @@ -71,6 +72,7 @@ class ARROW_EXPORT DataBatch { | ||
| using detail::ParsedValueDesc; | ||
| int32_t batch_row = 0; | ||
| size_t missing_index = 0; | ||
| for (size_t buf_index = 0; buf_index < values_buffers_.size(); ++buf_index) { | ||
| const auto& values_buffer = values_buffers_[buf_index]; | ||
| const auto values = reinterpret_cast<const ParsedValueDesc*>(values_buffer->data()); | ||
| @@ -80,7 +82,16 @@ class ARROW_EXPORT DataBatch { | ||
| auto start = values[pos].offset; | ||
| auto stop = values[pos + 1].offset; | ||
| auto quoted = values[pos + 1].quoted; | ||
| Status status = visit(parsed_ + start, stop - start, quoted); | ||
| const bool row_has_missing_fields = | ||
| missing_index < missing_fields_.size() && | ||
| missing_fields_[missing_index].row == batch_row; | ||
| const bool missing = | ||
| row_has_missing_fields && | ||
| col_index >= missing_fields_[missing_index].first_missing_column; | ||
| if (row_has_missing_fields) { | ||
| ++missing_index; | ||
| } | ||
| Status status = visit(parsed_ + start, stop - start, quoted, missing); | ||
HuaHuaY marked this conversation as resolved.
Uh oh!There was an error while loading. Please reload this page. | ||
| if (ARROW_PREDICT_FALSE(!status.ok())) { | ||
| return DecorateWithRowNumber(std::move(status), first_row, batch_row); | ||
| } | ||
| @@ -98,11 +109,15 @@ class ARROW_EXPORT DataBatch { | ||
| const auto start_pos = | ||
| static_cast<int32_t>(values_buffer->size() / sizeof(ParsedValueDesc)) - | ||
| num_cols_ - 1; | ||
| const bool last_row_has_missing_fields = | ||
| !missing_fields_.empty() && missing_fields_.back().row == num_rows_ - 1; | ||
| for (int32_t col_index = 0; col_index < num_cols_; ++col_index) { | ||
| auto start = values[start_pos + col_index].offset; | ||
| auto stop = values[start_pos + col_index + 1].offset; | ||
| auto quoted = values[start_pos + col_index + 1].quoted; | ||
| ARROW_RETURN_NOT_OK(visit(parsed_ + start, stop - start, quoted)); | ||
| const bool missing = last_row_has_missing_fields && | ||
| col_index >= missing_fields_.back().first_missing_column; | ||
| ARROW_RETURN_NOT_OK(visit(parsed_ + start, stop - start, quoted, missing)); | ||
| } | ||
| return Status::OK(); | ||
| } | ||
| @@ -138,6 +153,12 @@ class ARROW_EXPORT DataBatch { | ||
| // Record the current num_rows_ each time a row is skipped | ||
| std::vector<int32_t> skipped_rows_; | ||
| // Record the first missing column for rows padded with nulls | ||
| struct MissingFieldRange { | ||
| int32_t row; | ||
| int32_t first_missing_column; | ||
| }; | ||
| std::vector<MissingFieldRange> missing_fields_; | ||
| friend class ::arrow::csv::BlockParserImpl; | ||
| }; | ||
| @@ -206,7 +227,7 @@ class ARROW_EXPORT BlockParser { | ||
| /// \brief Visit parsed values in a column | ||
| /// | ||
| /// The signature of the visitor is | ||
| /// Status(const uint8_t* data, uint32_t size, bool quoted) | ||
| /// Status(const uint8_t* data, uint32_t size, bool quoted, bool missing) | ||
| template <typename Visitor> | ||
| Status VisitColumn(int32_t col_index, Visitor&& visit) const { | ||
| return parsed_batch().VisitColumn(col_index, first_row_num(), | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -91,7 +91,9 @@ void GetColumn(const BlockParser& parser, int32_t col_index, | ||
| std::vector<std::string>* out, std::vector<bool>* out_quoted = nullptr) { | ||
| std::vector<std::string> values; | ||
| std::vector<bool> quoted_values; | ||
| auto visit = [&](const uint8_t* data, uint32_t size, bool quoted) -> Status { | ||
| auto visit = [&](const uint8_t* data, uint32_t size, bool quoted, | ||
| bool missing) -> Status { | ||
HuaHuaY marked this conversation as resolved.
Uh oh!There was an error while loading. Please reload this page. | ||
| EXPECT_FALSE(missing); | ||
| values.push_back(std::string(reinterpret_cast<const char*>(data), size)); | ||
| if (out_quoted) { | ||
| quoted_values.push_back(quoted); | ||
| @@ -109,7 +111,9 @@ void GetLastRow(const BlockParser& parser, std::vector<std::string>* out, | ||
| std::vector<bool>* out_quoted = nullptr) { | ||
| std::vector<std::string> values; | ||
| std::vector<bool> quoted_values; | ||
| auto visit = [&](const uint8_t* data, uint32_t size, bool quoted) -> Status { | ||
| auto visit = [&](const uint8_t* data, uint32_t size, bool quoted, | ||
| bool missing) -> Status { | ||
HuaHuaY marked this conversation as resolved.
Uh oh!There was an error while loading. Please reload this page. | ||
| EXPECT_FALSE(missing); | ||
| values.push_back(std::string(reinterpret_cast<const char*>(data), size)); | ||
| if (out_quoted) { | ||
| quoted_values.push_back(quoted); | ||
| @@ -264,6 +268,36 @@ TEST(BlockParser, Basics) { | ||
| } | ||
| } | ||
| TEST(BlockParser, PadShortRows) { | ||
| auto options = ParseOptions::Defaults(); | ||
| options.pad_short_rows = true; | ||
| 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})); | ||
| } | ||
| TEST(BlockParser, EmptyHeader) { | ||
| // Cannot infer number of columns | ||
| uint32_t out_size; | ||
| @@ -884,10 +918,12 @@ TEST(BlockParser, RowNumberAppendedToError) { | ||
| BlockParser parser(options, -1, 0); | ||
| ASSERT_NO_FATAL_FAILURE(AssertParseOk(parser, csv)); | ||
| int row = 0; | ||
| auto status = parser.VisitColumn( | ||
| 0, [row](const uint8_t* data, uint32_t size, bool quoted) mutable -> Status { | ||
| return ++row == 2 ? Status::Invalid("Bad value") : Status::OK(); | ||
| }); | ||
| auto status = parser.VisitColumn(0, | ||
| [row](const uint8_t* data, uint32_t size, | ||
| bool quoted, bool missing) mutable -> Status { | ||
| return ++row == 2 ? Status::Invalid("Bad value") | ||
| : Status::OK(); | ||
| }); | ||
| EXPECT_RAISES_WITH_MESSAGE_THAT(Invalid, testing::HasSubstr("Row #1: Bad value"), | ||
| status); | ||
| } | ||
| @@ -896,10 +932,12 @@ TEST(BlockParser, RowNumberAppendedToError) { | ||
| BlockParser parser(options, -1, 100); | ||
| ASSERT_NO_FATAL_FAILURE(AssertParseOk(parser, csv)); | ||
| int row = 0; | ||
| auto status = parser.VisitColumn( | ||
| 0, [row](const uint8_t* data, uint32_t size, bool quoted) mutable -> Status { | ||
| return ++row == 3 ? Status::Invalid("Bad value") : Status::OK(); | ||
| }); | ||
| auto status = parser.VisitColumn(0, | ||
| [row](const uint8_t* data, uint32_t size, | ||
| bool quoted, bool missing) mutable -> Status { | ||
| return ++row == 3 ? Status::Invalid("Bad value") | ||
| : Status::OK(); | ||
| }); | ||
| EXPECT_RAISES_WITH_MESSAGE_THAT(Invalid, testing::HasSubstr("Row #102: Bad value"), | ||
| status); | ||
| } | ||
| @@ -909,10 +947,12 @@ TEST(BlockParser, RowNumberAppendedToError) { | ||
| BlockParser parser(options, -1, -1); | ||
| ASSERT_NO_FATAL_FAILURE(AssertParseOk(parser, csv)); | ||
| int row = 0; | ||
| auto status = parser.VisitColumn( | ||
| 0, [row](const uint8_t* data, uint32_t size, bool quoted) mutable -> Status { | ||
| return ++row == 3 ? Status::Invalid("Bad value") : Status::OK(); | ||
| }); | ||
| auto status = parser.VisitColumn(0, | ||
| [row](const uint8_t* data, uint32_t size, | ||
| bool quoted, bool missing) mutable -> Status { | ||
| return ++row == 3 ? Status::Invalid("Bad value") | ||
| : Status::OK(); | ||
| }); | ||
| EXPECT_RAISES_WITH_MESSAGE_THAT(Invalid, testing::Not(testing::HasSubstr("Row")), | ||
| status); | ||
| } | ||
| @@ -926,10 +966,12 @@ TEST(BlockParser, RowNumberAppendedToError) { | ||
| BlockParser parser(opts, /*num_cols=*/2, /*first_row=*/1); | ||
| ASSERT_NO_FATAL_FAILURE(AssertParseOk(parser, "a,b,c\nd,e\nf,g\nh\ni\nj,k\nl\n")); | ||
| int row = 0; | ||
| auto status = parser.VisitColumn( | ||
| 0, [row](const uint8_t* data, uint32_t size, bool quoted) mutable -> Status { | ||
| return ++row == 3 ? Status::Invalid("Bad value") : Status::OK(); | ||
| }); | ||
| auto status = parser.VisitColumn(0, | ||
| [row](const uint8_t* data, uint32_t size, | ||
| bool quoted, bool missing) mutable -> Status { | ||
| return ++row == 3 ? Status::Invalid("Bad value") | ||
| : Status::OK(); | ||
| }); | ||
| EXPECT_RAISES_WITH_MESSAGE_THAT(Invalid, testing::HasSubstr("Row #6: Bad value"), | ||
| status); | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -622,7 +622,9 @@ class ReaderMixin { | ||
| column_names_ = GenerateColumnNames(parser.num_cols()); | ||
| } else { | ||
| // Read column names from header row | ||
| auto visit = [&](const uint8_t* data, uint32_t size, bool quoted) -> Status { | ||
| auto visit = [&](const uint8_t* data, uint32_t size, bool quoted, | ||
| bool missing) -> Status { | ||
| DCHECK(!missing); | ||
| column_names_.emplace_back(reinterpret_cast<const char*>(data), size); | ||
HuaHuaY marked this conversation as resolved.
Uh oh!There was an error while loading. Please reload this page. | ||
| return Status::OK(); | ||
| }; | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -19,6 +19,7 @@ | ||
| #include <algorithm> | ||
| #include <memory> | ||
| #include <optional> | ||
| #include <sstream> | ||
| #include <string> | ||
| #include <unordered_set> | ||
| @@ -161,7 +162,16 @@ Result<std::vector<std::string>> GetOrderedColumnNames( | ||
| uint32_t parsed_size = 0; | ||
| int32_t max_num_rows = read_options.skip_rows + 1; | ||
| csv::BlockParser parser(pool, parse_options, /*num_cols=*/-1, /*first_row=*/1, | ||
| std::optional<csv::ParseOptions> inspection_parse_options; | ||
| const auto* parser_options = &parse_options; | ||
| if (parse_options.pad_short_rows) { | ||
HuaHuaY marked this conversation as resolved.
Uh oh!There was an error while loading. Please reload this page. | ||
| // Do not pad short rows while determining column names, since padding cannot | ||
| // synthesize missing names. Copy the parse options only when needed. | ||
| inspection_parse_options.emplace(parse_options); | ||
| inspection_parse_options->pad_short_rows = false; | ||
| parser_options = &*inspection_parse_options; | ||
| } | ||
| csv::BlockParser parser(pool, *parser_options, /*num_cols=*/-1, /*first_row=*/1, | ||
| max_num_rows); | ||
| RETURN_NOT_OK(parser.Parse(std::string_view{first_block}, &parsed_size)); | ||
| @@ -188,8 +198,9 @@ Result<std::vector<std::string>> GetOrderedColumnNames( | ||
| return column_names; | ||
| } | ||
| RETURN_NOT_OK( | ||
| parser.VisitLastRow([&](const uint8_t* data, uint32_t size, bool quoted) -> Status { | ||
| RETURN_NOT_OK(parser.VisitLastRow( | ||
| [&](const uint8_t* data, uint32_t size, bool quoted, bool missing) -> Status { | ||
| DCHECK(!missing); | ||
| std::string_view view{reinterpret_cast<const char*>(data), size}; | ||
| column_names.emplace_back(view); | ||
| return Status::OK(); | ||
| @@ -367,7 +378,8 @@ bool CsvFileFormat::Equals(const FileFormat& format) const { | ||
| parse_options.escaping == other_parse_options.escaping && | ||
| parse_options.escape_char == other_parse_options.escape_char && | ||
| parse_options.newlines_in_values == other_parse_options.newlines_in_values && | ||
| parse_options.ignore_empty_lines == other_parse_options.ignore_empty_lines; | ||
| parse_options.ignore_empty_lines == other_parse_options.ignore_empty_lines && | ||
| parse_options.pad_short_rows == other_parse_options.pad_short_rows; | ||
| } | ||
| Result<bool> CsvFileFormat::IsSupported(const FileSource& source) const { | ||
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.