Address preliminary code comments. Check in missing files

Change-Id: I8f0a35ae4e8581790f7731ee2ed023a54caf0f31

cpp/src/arrow/array-binary-test.cc

@@ -754,6 +754,14 @@ TEST_F(TestChunkedBinaryBuilder, LargeElements) {
   ArrayVector chunks;
   ASSERT_OK(builder_->Finish(&chunks));
   ASSERT_EQ(iterations, static_cast<int>(chunks.size()));
+
+  int64_t total_data_size = 0;
+  for (auto chunk : chunks) {
+    ASSERT_EQ(1, chunk->length());
+    total_data_size +=
+        static_cast<int64_t>(static_cast<const BinaryArray&>(*chunk).GetView(0).size());
+  }
+  ASSERT_EQ(iterations * bufsize, total_data_size);
 }
 
 TEST(TestChunkedStringBuilder, BasicOperation) {

cpp/src/arrow/columnar/CMakeLists.txt

@@ -0,0 +1,27 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+# Headers: top level
+install(FILES
+  builder_adaptive.h
+  builder_base.h
+  builder_binary.h
+  builder_decimal.h
+  builder_dict.h
+  builder_nested.h
+  builder_primitive.h
+  DESTINATION "${CMAKE_INSTALL_INCLUDEDIR}/arrow/columnar")

cpp/src/arrow/columnar/builder_binary.cc

@@ -305,17 +305,13 @@ Status ChunkedBinaryBuilder::NextChunk() {
 }
 
 Status ChunkedStringBuilder::Finish(ArrayVector* out) {
-  ArrayVector temp;
-  RETURN_NOT_OK(ChunkedBinaryBuilder::Finish(&temp));
+  RETURN_NOT_OK(ChunkedBinaryBuilder::Finish(out));
 
-  out->clear();
-  out->reserve(temp.size());
-
-  // XXX: Change data type to string/utf8
-  for (const auto& chunk : temp) {
-    std::shared_ptr<ArrayData> data = chunk->data();
+  // Change data type to string/utf8
+  for (size_t i = 0; i < out->size(); ++i) {
+    std::shared_ptr<ArrayData> data = (*out)[i]->data();
     data->type = ::arrow::utf8();
-    out->emplace_back(std::make_shared<StringArray>(data));
+    (*out)[i] = std::make_shared<StringArray>(data);
   }
   return Status::OK();
 }

cpp/src/arrow/columnar/builder_nested.h

@@ -113,12 +113,9 @@ class ARROW_EXPORT StructBuilder : public ArrayBuilder {
 
   void Reset() override;
 
-  ArrayBuilder* field_builder(int i) const { return field_builders_[i].get(); }
+  ArrayBuilder* field_builder(int i) const { return children_[i].get(); }
 
-  int num_fields() const { return static_cast<int>(field_builders_.size()); }
-
- protected:
-  std::vector<std::shared_ptr<ArrayBuilder>> field_builders_;
+  int num_fields() const { return static_cast<int>(children_.size()); }
 };
 
 }  // namespace arrow

cpp/src/arrow/compute/kernel.h

@@ -80,8 +80,7 @@ struct ARROW_EXPORT Datum {
 
   // Cast from subtypes of Array to Datum
   template <typename T,
-            typename = typename std::enable_if<std::is_base_of<Array, T>::value,
-                                               void>::type>
+            typename = typename std::enable_if<std::is_base_of<Array, T>::value>::type>
   Datum(const std::shared_ptr<T>& value)  // NOLINT implicit conversion
       : Datum(std::shared_ptr<Array>(value)) {}
 

cpp/src/parquet/arrow/reader.cc

@@ -32,7 +32,7 @@
 #include "arrow/util/logging.h"
 #include "arrow/util/thread-pool.h"
 
-// XXX(wesm): For arrow::compute::Datum. This should perhaps be promoted
+// For arrow::compute::Datum. This should perhaps be promoted. See ARROW-4022
 #include "arrow/compute/kernel.h"
 
 #include "parquet/arrow/record_reader.h"