Skip to content
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

Make CSV block size configurable. #42

Merged
merged 5 commits into from
Jul 2, 2024
Merged

Make CSV block size configurable. #42

merged 5 commits into from
Jul 2, 2024

Conversation

elefeint
Copy link
Contributor

The default block size in Arrow CSV reader is 1MB (or so?). This PR makes it configurable in multiples of a megabyte.

Fixes ECO-142

@elefeint elefeint requested a review from hrl20 June 28, 2024 22:14
auto parse_options = arrow::csv::ParseOptions::Defaults();
auto convert_options = get_arrow_convert_options(utf8_columns, null_value);
parse_options.newlines_in_values = true;
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sneaking this setting in since it came up in my testing, and I don't see why you'd ever want this to be off.

auto msg = "Authentication test for database <" + db_name +
"> failed: " + std::string(e.what());
auto msg =
"Test for database <" + db_name + "> failed: " + std::string(e.what());
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe include the test name in msg

}

{
// succeed when block_size is increased
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nice test!!

@elefeint elefeint merged commit 996ec23 into main Jul 2, 2024
1 check passed
@elefeint elefeint deleted the csv_block_size branch July 2, 2024 02:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants