ARROW-6158: [C++/Python] Validate child array types with type fields of StructArray

jorisvandenbossche · wesm · commit 199d3cf9c8a1 · 2019-09-24T22:02:07.000-05:00
https://issues.apache.org/jira/browse/ARROW-6158 Closes apache#5488 from jorisvandenbossche/ARROW-6158-struct-array-validation and squashes the following commits: 7573781 <Joris Van den Bossche> ARROW-6158: Validate child array types with type fields of StructArray Authored-by: Joris Van den Bossche <jorisvandenbossche@gmail.com> Signed-off-by: Wes McKinney <wesm+git@apache.org>
diff --git a/cpp/src/arrow/array.cc b/cpp/src/arrow/array.cc
@@ -1234,6 +1234,7 @@ struct ValidateVisitor {
   }
 
   Status Visit(const StructArray& array) {
+    const auto& struct_type = checked_cast<const StructType&>(*array.type());
     if (array.num_fields() > 0) {
       // Validate fields
       int64_t array_length = array.field(0)->length();
@@ -1245,10 +1246,17 @@ struct ValidateVisitor {
                                  it->type()->ToString(), " at position [", idx, "]");
         }
 
+        auto it_type = struct_type.child(i)->type();
+        if (!it->type()->Equals(it_type)) {
+          return Status::Invalid("Child array at position [", idx,
+                                 "] does not match type field: ", it->type()->ToString(),
+                                 " vs ", it_type->ToString());
+        }
+
         const Status child_valid = it->Validate();
         if (!child_valid.ok()) {
           return Status::Invalid("Child array invalid: ", child_valid.ToString(),
-                                 " at position [", idx, "}");
+                                 " at position [", idx, "]");
         }
         ++idx;
       }
diff --git a/python/pyarrow/array.pxi b/python/pyarrow/array.pxi
@@ -1288,7 +1288,9 @@ cdef class UnionArray(Array):
             check_status(CUnionArray.MakeDense(
                 deref(types.ap), deref(value_offsets.ap), c, c_field_names,
                 c_type_codes, &out))
-        return pyarrow_wrap_array(out)
+        cdef Array result = pyarrow_wrap_array(out)
+        result.validate()
+        return result
 
     @staticmethod
     def from_sparse(Array types, list children, list field_names=None,
@@ -1326,7 +1328,9 @@ cdef class UnionArray(Array):
                                                 c_field_names,
                                                 c_type_codes,
                                                 &out))
-        return pyarrow_wrap_array(out)
+        cdef Array result = pyarrow_wrap_array(out)
+        result.validate()
+        return result
 
 
 cdef class StringArray(Array):
@@ -1501,7 +1505,9 @@ cdef class DictionaryArray(Array):
             c_result.reset(new CDictionaryArray(c_type, _indices.sp_array,
                                                 _dictionary.sp_array))
 
-        return pyarrow_wrap_array(c_result)
+        cdef Array result = pyarrow_wrap_array(c_result)
+        result.validate()
+        return result
 
 
 cdef class StructArray(Array):
@@ -1626,7 +1632,9 @@ cdef class StructArray(Array):
         else:
             c_result = CStructArray.MakeFromFields(
                 c_arrays, c_fields, shared_ptr[CBuffer](), -1, 0)
-        return pyarrow_wrap_array(GetResultValue(c_result))
+        cdef Array result = pyarrow_wrap_array(GetResultValue(c_result))
+        result.validate()
+        return result
 
 
 cdef class ExtensionArray(Array):
@@ -1665,7 +1673,9 @@ cdef class ExtensionArray(Array):
                             "for extension type {1}".format(storage.type, typ))
 
         ext_array = make_shared[CExtensionArray](typ.sp_type, storage.sp_array)
-        return pyarrow_wrap_array(<shared_ptr[CArray]> ext_array)
+        cdef Array result = pyarrow_wrap_array(<shared_ptr[CArray]> ext_array)
+        result.validate()
+        return result
 
 
 cdef dict _array_classes = {
diff --git a/python/pyarrow/tests/test_array.py b/python/pyarrow/tests/test_array.py
@@ -422,7 +422,7 @@ def test_struct_from_buffers():
 
 
 def test_struct_from_arrays():
-    a = pa.array([4, 5, 6])
+    a = pa.array([4, 5, 6], type=pa.int64())
     b = pa.array(["bar", None, ""])
     c = pa.array([[1, 2], None, [3, None]])
     expected_list = [
@@ -447,7 +447,7 @@ def test_struct_from_arrays():
     # From fields
     fa = pa.field("a", a.type, nullable=False)
     fb = pa.field("b", b.type)
-    fc = pa.field("c", b.type)
+    fc = pa.field("c", c.type)
     arr = pa.StructArray.from_arrays([a, b, c], fields=[fa, fb, fc])
     assert arr.type == pa.struct([fa, fb, fc])
     assert not arr.type[0].nullable
@@ -460,6 +460,11 @@ def test_struct_from_arrays():
     assert arr.type == pa.struct([])
     assert arr.to_pylist() == []
 
+    # Inconsistent fields
+    fa2 = pa.field("a", pa.int32())
+    with pytest.raises(ValueError, match="int64 vs int32"):
+        pa.StructArray.from_arrays([a, b, c], fields=[fa2, fb, fc])
+
 
 def test_dictionary_from_numpy():
     indices = np.repeat([0, 1, 2], 2)

Original file line number	Diff line number	Diff line change
`@@ -1234,6 +1234,7 @@ struct ValidateVisitor {`
`1234`	`1234`	`}`
`1235`	`1235`
`1236`	`1236`	`Status Visit(const StructArray& array) {`
	`1237`	`+ const auto& struct_type = checked_cast<const StructType&>(*array.type());`
`1237`	`1238`	`if (array.num_fields() > 0) {`
`1238`	`1239`	`// Validate fields`
`1239`	`1240`	`int64_t array_length = array.field(0)->length();`
`@@ -1245,10 +1246,17 @@ struct ValidateVisitor {`
`1245`	`1246`	`it->type()->ToString(), " at position [", idx, "]");`
`1246`	`1247`	`}`
`1247`	`1248`
	`1249`	`+ auto it_type = struct_type.child(i)->type();`
	`1250`	`+ if (!it->type()->Equals(it_type)) {`
	`1251`	`+ return Status::Invalid("Child array at position [", idx,`
	`1252`	`+ "] does not match type field: ", it->type()->ToString(),`
	`1253`	`+ " vs ", it_type->ToString());`
	`1254`	`+ }`
	`1255`	`+`
`1248`	`1256`	`const Status child_valid = it->Validate();`
`1249`	`1257`	`if (!child_valid.ok()) {`
`1250`	`1258`	`return Status::Invalid("Child array invalid: ", child_valid.ToString(),`
`1251`		`- " at position [", idx, "}");`
	`1259`	`+ " at position [", idx, "]");`
`1252`	`1260`	`}`
`1253`	`1261`	`++idx;`
`1254`	`1262`	`}`