lidavidm commented on a change in pull request #12706:
URL: https://github.com/apache/arrow/pull/12706#discussion_r834439910
##########
File path: cpp/src/arrow/table_test.cc
##########
@@ -56,6 +56,22 @@ class TestTable : public ::testing::Test {
std::make_shared<ChunkedArray>(arrays_[2])};
}
+ void MakeExample3(int length) {
+ auto f0 = field("f0", int32(), false);
+ auto f1 = field("f1", uint8());
+ auto f2 = field("f2", int16());
+
+ std::vector<std::shared_ptr<Field>> fields = {f0, f1, f2};
+ schema_ = std::make_shared<Schema>(fields);
+
+ arrays_ = {gen_.ArrayOf(int32(), length, true), gen_.ArrayOf(uint8(),
length),
Review comment:
The third parameter here `null_probability` is a `double`, so maybe use
`1.0` instead of `true`?
##########
File path: cpp/src/arrow/table.cc
##########
@@ -200,6 +200,9 @@ class SimpleTable : public Table {
ss << "Column " << i << ": " << st.message();
return st.WithMessage(ss.str());
}
+ if (!schema_->field(i)->nullable() && col->null_count() > 0){
+ return Status::Invalid("In column ", i, ": Null found but expected non
nullable ");
Review comment:
```suggestion
return Status::Invalid("In column ", i, ": Null found but field is
not nullable");
```
##########
File path: cpp/src/arrow/record_batch_test.cc
##########
@@ -89,6 +89,7 @@ TEST_F(TestRecordBatch, Validate) {
auto a1 = gen.ArrayOf(uint8(), length);
auto a2 = gen.ArrayOf(int16(), length);
auto a3 = gen.ArrayOf(int16(), 5);
+ auto a4 = gen.ArrayOf(int32(), length, true);
Review comment:
```suggestion
auto a4 = gen.ArrayOf(int32(), length, /*null_probability=*/1.0);
```
##########
File path: cpp/src/arrow/table_test.cc
##########
@@ -65,6 +81,13 @@ class TestTable : public ::testing::Test {
std::vector<std::shared_ptr<ChunkedArray>> columns_;
};
+TEST_F(TestTable, ValidateNullable) {
+ const int length = 10;
+ MakeExample3(length);
+ table_ = Table::Make(schema_, columns_);
+ ASSERT_RAISES(Invalid, table_->ValidateFull());
Review comment:
We can be more precise:
##########
File path: cpp/src/arrow/table_test.cc
##########
@@ -56,6 +56,22 @@ class TestTable : public ::testing::Test {
std::make_shared<ChunkedArray>(arrays_[2])};
}
+ void MakeExample3(int length) {
+ auto f0 = field("f0", int32(), false);
+ auto f1 = field("f1", uint8());
+ auto f2 = field("f2", int16());
+
+ std::vector<std::shared_ptr<Field>> fields = {f0, f1, f2};
+ schema_ = std::make_shared<Schema>(fields);
+
+ arrays_ = {gen_.ArrayOf(int32(), length, true), gen_.ArrayOf(uint8(),
length),
Review comment:
```suggestion
arrays_ = {gen_.ArrayOf(int32(), length, /*null_probability=*/1.0),
gen_.ArrayOf(uint8(), length),
```
##########
File path: cpp/src/arrow/record_batch_test.cc
##########
@@ -89,6 +89,7 @@ TEST_F(TestRecordBatch, Validate) {
auto a1 = gen.ArrayOf(uint8(), length);
auto a2 = gen.ArrayOf(int16(), length);
auto a3 = gen.ArrayOf(int16(), 5);
+ auto a4 = gen.ArrayOf(int32(), length, true);
Review comment:
Same here
##########
File path: cpp/src/arrow/record_batch.cc
##########
@@ -313,6 +313,9 @@ Status ValidateBatch(const RecordBatch& batch, bool
full_validation) {
if (!st.ok()) {
return Status::Invalid("In column ", i, ": ", st.ToString());
}
+ if (full_validation && !batch.schema()->field(i)->nullable() &&
array.null_count() > 0) {
+ return Status::Invalid("In column ", i, ": Null found but expected non
nullable ");
Review comment:
```suggestion
return Status::Invalid("In column ", i, ": Null found but field is not
nullable");
```
##########
File path: cpp/src/arrow/table_test.cc
##########
@@ -65,6 +81,13 @@ class TestTable : public ::testing::Test {
std::vector<std::shared_ptr<ChunkedArray>> columns_;
};
+TEST_F(TestTable, ValidateNullable) {
+ const int length = 10;
+ MakeExample3(length);
+ table_ = Table::Make(schema_, columns_);
+ ASSERT_RAISES(Invalid, table_->ValidateFull());
Review comment:
(and ditto above)
##########
File path: cpp/src/arrow/table_test.cc
##########
@@ -65,6 +81,13 @@ class TestTable : public ::testing::Test {
std::vector<std::shared_ptr<ChunkedArray>> columns_;
};
+TEST_F(TestTable, ValidateNullable) {
+ const int length = 10;
+ MakeExample3(length);
+ table_ = Table::Make(schema_, columns_);
+ ASSERT_RAISES(Invalid, table_->ValidateFull());
Review comment:
```suggestion
EXPECT_RAISES_WITH_MESSAGE_THAT(Invalid, ::testing::HasSubstr("In column
0: Null found but field is not nullable"), table_->ValidateFull());
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
