ARROW-6825: [C++] Rework CSV reader IO around readahead iterator

[pitrou]

Make the delimiting chunker a common facility used by CSV and JSON.

[pitrou]

@bkietz

[github-actions]

https://issues.apache.org/jira/browse/ARROW-6825

[pitrou]

@kou There is a weird Github Actions error here: https://github.com/apache/arrow/pull/5727/checks?check_run_id=273513110

Looks like GH uses powershell by default?

Edit: indeed: https://github.blog/changelog/2019-10-17-github-actions-default-shell-on-windows-runners-is-changing-to-powershell/

[bkietz]

Very elegant. Just a few comments

[bkietz]

This is empty enough to fold into json/reader.cc

[pitrou]

chunker.h is small, but chunker.cc is non-trivial IMHO.

[pitrou]

@bkietz I think I've addressed all your comments.
@fsaintjacques Are you still willing to review?

[bkietz]

LGTM

[pprudhvi]

@pitrou I'm getting 'ambiguous function call' compilation errors with this and AssertParseOk(line 241). Not sure why this was not caught in CI builds. I am building with tests, gandiva, jni ON. Could you please take a look and let me know if I should set any compiler flags? Thanks.

[pitrou]

Can you post the full compiler output?

[pprudhvi]

`
/Users/travis/build/dremio/arrow-build/arrow/cpp/src/arrow/csv/parser_test.cc:241:5: error: call to 'AssertParseOk' is ambiguous

AssertParseOk(parser, {csv1, csv2});

^~~~~~~~~~~~~

/Users/travis/build/dremio/arrow-build/arrow/cpp/src/arrow/csv/parser_test.cc:138:6: note: candidate function

void AssertParseOk(BlockParser& parser, const std::string& str) {

 ^

/Users/travis/build/dremio/arrow-build/arrow/cpp/src/arrow/csv/parser_test.cc:144:6: note: candidate function

void AssertParseOk(BlockParser& parser, const std::vectorutil::string_view& data) {

 ^

/Users/travis/build/dremio/arrow-build/arrow/cpp/src/arrow/csv/parser_test.cc:393:3: error: call to 'AssertParseFinal' is ambiguous

AssertParseFinal(parser, {csv1, csv2});

^~~~~~~~~~~~~~~~

/Users/travis/build/dremio/arrow-build/arrow/cpp/src/arrow/csv/parser_test.cc:150:6: note: candidate function

void AssertParseFinal(BlockParser& parser, const std::string& str) {

 ^

/Users/travis/build/dremio/arrow-build/arrow/cpp/src/arrow/csv/parser_test.cc:156:6: note: candidate function

void AssertParseFinal(BlockParser& parser, const std::vectorutil::string_view& data) {

 ^

`

[pitrou]

Can you try the following patch?

diff --git a/cpp/src/arrow/csv/parser_test.cc b/cpp/src/arrow/csv/parser_test.cc
index f988f3ce2..418340f54 100644
--- a/cpp/src/arrow/csv/parser_test.cc
+++ b/cpp/src/arrow/csv/parser_test.cc
@@ -135,16 +135,25 @@ Status ParseFinal(BlockParser& parser, const std::string& str, uint32_t* out_siz
   return parser.ParseFinal(util::string_view(str), out_size);
 }
 
+std::vector<util::string_view> ViewsFromStrings(const std::vector<std::string>& data) {
+  std::vector<util::string_view> views(data.size());
+  for (size_t i = 0; i < data.size(); ++i) {
+    views[i] = data[i];
+  }
+  return views;
+}
+
 void AssertParseOk(BlockParser& parser, const std::string& str) {
   uint32_t parsed_size = static_cast<uint32_t>(-1);
   ASSERT_OK(Parse(parser, str, &parsed_size));
   ASSERT_EQ(parsed_size, str.size());
 }
 
-void AssertParseOk(BlockParser& parser, const std::vector<util::string_view>& data) {
+void AssertParseOk(BlockParser& parser, const std::vector<std::string>& data) {
   uint32_t parsed_size = static_cast<uint32_t>(-1);
-  ASSERT_OK(parser.Parse(data, &parsed_size));
-  ASSERT_EQ(parsed_size, TotalViewLength(data));
+  auto views = ViewsFromStrings(data);
+  ASSERT_OK(parser.Parse(views, &parsed_size));
+  ASSERT_EQ(parsed_size, TotalViewLength(views));
 }
 
 void AssertParseFinal(BlockParser& parser, const std::string& str) {
@@ -153,10 +162,11 @@ void AssertParseFinal(BlockParser& parser, const std::string& str) {
   ASSERT_EQ(parsed_size, str.size());
 }
 
-void AssertParseFinal(BlockParser& parser, const std::vector<util::string_view>& data) {
+void AssertParseFinal(BlockParser& parser, const std::vector<std::string>& data) {
   uint32_t parsed_size = static_cast<uint32_t>(-1);
-  ASSERT_OK(parser.ParseFinal(data, &parsed_size));
-  ASSERT_EQ(parsed_size, TotalViewLength(data));
+  auto views = ViewsFromStrings(data);
+  ASSERT_OK(parser.ParseFinal(views, &parsed_size));
+  ASSERT_EQ(parsed_size, TotalViewLength(views));
 }
 
 void AssertParsePartial(BlockParser& parser, const std::string& str,

[pprudhvi]

same error with the patch

[pprudhvi]

enclosing elements of initializer list in braces seems to work. I pushed a patch - https://github.com/apache/arrow/pull/5791/files . Please merge if it looks alright. Thanks.