You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@arrow.apache.org by uw...@apache.org on 2019/02/12 14:29:07 UTC

[arrow] branch master updated: ARROW-4535: [C++] Fix MakeBuilder to preserve ListType's field name

This is an automated email from the ASF dual-hosted git repository.

uwe pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/arrow.git


The following commit(s) were added to refs/heads/master by this push:
     new 35b2871  ARROW-4535: [C++] Fix MakeBuilder to preserve ListType's field name
35b2871 is described below

commit 35b28719fbf2fe7bea47f17ee93bd771020bd9ce
Author: Kenta Murata <mr...@mrkn.jp>
AuthorDate: Tue Feb 12 15:28:56 2019 +0100

    ARROW-4535: [C++] Fix MakeBuilder to preserve ListType's field name
    
    MakeBuilder doesn't preserve the field name in the given ListType.
    I think this is a bug.
    
    Author: Kenta Murata <mr...@mrkn.jp>
    
    Closes #3619 from mrkn/fix_make_builder_to_preserve_field_name_of_list_type and squashes the following commits:
    
    e4d85978 <Kenta Murata> Fix c_glib's test
    8fc747a6 <Kenta Murata> Fix MakeBuilder to preserve ListType's field name
---
 c_glib/test/helper/buildable.rb     |  4 ++--
 c_glib/test/test-list-array.rb      |  2 +-
 c_glib/test/test-orc-file-reader.rb |  4 ++--
 cpp/src/arrow/array-list-test.cc    | 17 +++++++++++++++++
 cpp/src/arrow/builder.cc            |  2 +-
 5 files changed, 23 insertions(+), 6 deletions(-)

diff --git a/c_glib/test/helper/buildable.rb b/c_glib/test/helper/buildable.rb
index f3ae709..a9c514c 100644
--- a/c_glib/test/helper/buildable.rb
+++ b/c_glib/test/helper/buildable.rb
@@ -101,8 +101,8 @@ module Helper
       build_array(Arrow::StringArrayBuilder.new, values)
     end
 
-    def build_list_array(value_data_type, values_list)
-      value_field = Arrow::Field.new("value", value_data_type)
+    def build_list_array(value_data_type, values_list, field_name: "value")
+      value_field = Arrow::Field.new(field_name, value_data_type)
       data_type = Arrow::ListDataType.new(value_field)
       builder = Arrow::ListArrayBuilder.new(data_type)
       values_list.each do |values|
diff --git a/c_glib/test/test-list-array.rb b/c_glib/test/test-list-array.rb
index abbd004..eea16cc 100644
--- a/c_glib/test/test-list-array.rb
+++ b/c_glib/test/test-list-array.rb
@@ -19,7 +19,7 @@ class TestListArray < Test::Unit::TestCase
   include Helper::Buildable
 
   def test_new
-    field = Arrow::Field.new("item", Arrow::Int8DataType.new)
+    field = Arrow::Field.new("value", Arrow::Int8DataType.new)
     data_type = Arrow::ListDataType.new(field)
     value_offsets = Arrow::Buffer.new([0, 2, 5, 5].pack("l*"))
     data = Arrow::Buffer.new([1, 2, 3, 4, 5].pack("c*"))
diff --git a/c_glib/test/test-orc-file-reader.rb b/c_glib/test/test-orc-file-reader.rb
index 544112c..8b4f08f 100644
--- a/c_glib/test/test-orc-file-reader.rb
+++ b/c_glib/test/test-orc-file-reader.rb
@@ -64,7 +64,7 @@ map: list<item: struct<key: string, value: struct<int1: int32, string1: string>>
   end
 
   def build_items_array(items_array)
-    build_list_array(item_data_type, items_array)
+    build_list_array(item_data_type, items_array, field_name: "item")
   end
 
   def items_data_type
@@ -93,7 +93,7 @@ map: list<item: struct<key: string, value: struct<int1: int32, string1: string>>
   end
 
   def build_key_value_array(key_value_array)
-    build_list_array(key_value_data_type, key_value_array)
+    build_list_array(key_value_data_type, key_value_array, field_name: "item")
   end
 
   def middle_array
diff --git a/cpp/src/arrow/array-list-test.cc b/cpp/src/arrow/array-list-test.cc
index e616674..cb84896 100644
--- a/cpp/src/arrow/array-list-test.cc
+++ b/cpp/src/arrow/array-list-test.cc
@@ -296,4 +296,21 @@ TEST_F(TestListArray, TestZeroLength) {
   ASSERT_OK(ValidateArray(*result_));
 }
 
+TEST_F(TestListArray, TestBuilderPreserveFieleName) {
+  auto list_type_with_name = list(field("counts", int32()));
+
+  std::unique_ptr<ArrayBuilder> tmp;
+  ASSERT_OK(MakeBuilder(pool_, list_type_with_name, &tmp));
+  builder_.reset(checked_cast<ListBuilder*>(tmp.release()));
+
+  vector<int32_t> values = {1, 2, 4, 8};
+  ASSERT_OK(builder_->AppendValues(values.data(), values.size()));
+
+  std::shared_ptr<Array> list_array;
+  ASSERT_OK(builder_->Finish(&list_array));
+
+  const auto& type = checked_cast<ListType&>(*list_array->type());
+  ASSERT_EQ("counts", type.value_field()->name());
+}
+
 }  // namespace arrow
diff --git a/cpp/src/arrow/builder.cc b/cpp/src/arrow/builder.cc
index 2072edc..8d0ab19 100644
--- a/cpp/src/arrow/builder.cc
+++ b/cpp/src/arrow/builder.cc
@@ -75,7 +75,7 @@ Status MakeBuilder(MemoryPool* pool, const std::shared_ptr<DataType>& type,
       std::shared_ptr<DataType> value_type =
           internal::checked_cast<const ListType&>(*type).value_type();
       RETURN_NOT_OK(MakeBuilder(pool, value_type, &value_builder));
-      out->reset(new ListBuilder(pool, std::move(value_builder)));
+      out->reset(new ListBuilder(pool, std::move(value_builder), type));
       return Status::OK();
     }